PDA

View Full Version : [FIXED-992] Layout memory leaks



VT-TizianoF
26 May 2010, 5:51 AM
I've just found a couple of memory leaks on extjs, using sIEve. I'm not really sure about the overrides, anyone can check them?

TableLayout doesn't have a destroy method:

Ext.override(Ext.layout.TableLayout, {
destroy : function() {
Ext.layout.TableLayout.superclass.destroy.call(this);

Ext.destroy(Ext.get(this.table));
}
});If you have a split region, the x-tool-expand-position tool isn't destroyed

Ext.override(Ext.layout.BorderLayout.Region, {
destroy : function() {
if (this.autoHideSlideTask && this.autoHideSlideTask.cancel) {
this.autoHideSlideTask.cancel();
}
if (this.collapsedEl && this.collapsedEl.dom && this.collapsedEl.dom.firstChild) {
Ext.destroy(Ext.get(this.collapsedEl.dom.firstChild));
}
Ext.destroy(this.miniCollapsedEl, this.collapsedEl);
}
});Don't know why, but without the red line the x-splitbar-proxy-position-v tool leaks on a border layout

Ext.override(Ext.SplitBar, {
destroy : function(removeEl) {
Ext.destroy(this.shim, Ext.get(this.proxy));
this.proxy = null;
this.dd.unreg();
if (removeEl) {
this.el.remove();
}
this.purgeListeners();
}
});

jay@moduscreate.com
26 May 2010, 6:11 AM
What version of the framework? If so, this might be a good thing to move to bugs. Need to know the version though.

VT-TizianoF
26 May 2010, 6:13 AM
Latest one, 3.2.1

Condor
26 May 2010, 6:28 AM
For issue 2:

Region is not destroying the expand tool button. It would be better if Ext had a reference to it, e.g.

Ext.override(Ext.layout.BorderLayout.Region, {
getCollapsedEl : function(){
if(!this.collapsedEl){
if(!this.toolTemplate){
var tt = new Ext.Template(
'<div class="x-tool x-tool-{id}"> </div>'
);
tt.disableFormats = true;
tt.compile();
Ext.layout.BorderLayout.Region.prototype.toolTemplate = tt;
}
this.collapsedEl = this.targetEl.createChild({
cls: "x-layout-collapsed x-layout-collapsed-"+this.position,
id: this.panel.id + '-xcollapsed'
});
this.collapsedEl.enableDisplayMode('block');
if(this.collapseMode == 'mini'){
this.collapsedEl.addClass('x-layout-cmini-'+this.position);
this.miniCollapsedEl = this.collapsedEl.createChild({
cls: "x-layout-mini x-layout-mini-"+this.position, html: " "
});
this.miniCollapsedEl.addClassOnOver('x-layout-mini-over');
this.collapsedEl.addClassOnOver("x-layout-collapsed-over");
this.collapsedEl.on('click', this.onExpandClick, this, {stopEvent:true});
}else {
if(this.collapsible !== false && !this.hideCollapseTool) {
var t = this.expandToolEl = this.toolTemplate.append(
this.collapsedEl.dom,
{id:'expand-'+this.position}, true);
t.addClassOnOver('x-tool-expand-'+this.position+'-over');
t.on('click', this.onExpandClick, this, {stopEvent:true});
}
if(this.floatable !== false || this.titleCollapse){
this.collapsedEl.addClassOnOver("x-layout-collapsed-over");
this.collapsedEl.on("click", this[this.floatable ? 'collapseClick' : 'onExpandClick'], this);
}
}
}
return this.collapsedEl;
},
destroy : function(){
if (this.autoHideSlideTask && this.autoHideSlideTask.cancel){
this.autoHideSlideTask.cancel();
}
Ext.destroy(this.expandToolEl, this.miniCollapsedEl, this.collapsedEl);
}
});

evant
31 May 2010, 7:43 PM
1) I wasn't seeing the table layout leak, but it doesn't hurt to delete the reference.
2) Fixed.
3) It wasn't on that line, but it should be resolved now as well.