[FIX] web: Views: always destroy BasicModel instances

Before this rev., it might happen that the parent of BasicModel instances
wasn't correctly set to the Controller instance. It was actually set to the
ActionManager instance, which is never destroyed. So when this happened,
the BasicModel instance wasn't destroyed when it's controller was.
This commit is contained in:
Christophe Matthieu
2018-02-08 15:46:00 +01:00
parent 625a648f22
commit b28ceec009
6 changed files with 262 additions and 5 deletions
@@ -138,21 +138,23 @@ var AbstractView = Class.extend({
*/
getController: function (parent) {
var self = this;
// check if a model already exists, as if not, one will be created and
// we'll have to set the controller as its parent
var alreadyHasModel = !!this.model;
return $.when(this._loadData(parent), ajax.loadLibs(this)).then(function () {
var model = self.getModel();
var state = model.get(arguments[0]);
var state = self.model.get(arguments[0]);
var renderer = self.getRenderer(parent, state);
var Controller = self.Controller || self.config.Controller;
var controllerParams = _.extend({
initialState: state,
}, self.controllerParams);
var controller = new Controller(parent, model, renderer, controllerParams);
var controller = new Controller(parent, self.model, renderer, controllerParams);
renderer.setParent(controller);
if (!self.model) {
if (!alreadyHasModel) {
// if we have a model, it already has a parent. Otherwise, we
// set the controller, so the rpcs from the model actually work
model.setParent(controller);
self.model.setParent(controller);
}
return controller;
});
@@ -174,6 +174,7 @@ function createAsyncView(params) {
return view.getController(widget).then(function (view) {
// override the view's 'destroy' so that it calls 'destroy' on the widget
// instead, as the widget is the parent of the view and the mockServer.
view.__destroy = view.destroy;
view.destroy = function () {
// remove the override to properly destroy the view and its children
// when it will be called the second time (by its parent)
@@ -4,6 +4,7 @@ odoo.define('web.calendar_tests', function (require) {
var CalendarView = require('web.CalendarView');
var CalendarRenderer = require('web.CalendarRenderer');
var fieldUtils = require('web.field_utils');
var mixins = require('web.mixins');
var testUtils = require('web.test_utils');
var session = require('web.session');
@@ -1650,6 +1651,64 @@ QUnit.module('Views', {
calendar.destroy();
});
QUnit.test('check if the view destroys all widgets and instances', function (assert) {
assert.expect(1);
var instanceNumber = 0;
var initWidget = mixins.ParentedMixin.init;
mixins.ParentedMixin.init = function () {
instanceNumber++;
return initWidget.apply(this, arguments);
};
var destroyWidget = mixins.ParentedMixin.destroy;
mixins.ParentedMixin.destroy = function () {
if (!this.isDestroyed()) {
instanceNumber--;
}
return destroyWidget.apply(this, arguments);
};
var params = {
View: CalendarView,
model: 'event',
data: this.data,
arch:
'<calendar class="o_calendar_test" '+
'event_open_popup="true" '+
'date_start="start_date" '+
'all_day="allday" '+
'mode="week" '+
'attendee="partner_ids" '+
'color="partner_id">'+
'<field name="name"/>'+
'<filter name="user_id" avatar_field="image"/>'+
'<field name="partner_ids" write_model="filter_partner" write_field="partner_id"/>'+
'</calendar>',
archs: archs,
viewOptions: {
initialDate: initialDate,
},
};
var calendar = createView(params);
calendar.destroy();
var initialInstanceNumber = instanceNumber;
instanceNumber = 0;
calendar = createView(params);
// call destroy function of controller to ensure that it correctly destroys everything
calendar.__destroy();
assert.strictEqual(instanceNumber, initialInstanceNumber+1, "every widget must be destroyed exept the parent");
calendar.destroy();
mixins.ParentedMixin.init = initWidget;
mixins.ParentedMixin.destroy = destroyWidget;
});
});
});
@@ -6,6 +6,7 @@ var config = require('web.config');
var core = require('web.core');
var fieldRegistry = require('web.field_registry');
var FormView = require('web.FormView');
var mixins = require('web.mixins');
var pyeval = require('web.pyeval');
var RainbowMan = require('web.rainbow_man');
var testUtils = require('web.test_utils');
@@ -6337,5 +6338,84 @@ QUnit.module('Views', {
form.destroy();
});
QUnit.test('check if the view destroys all widgets and instances', function (assert) {
assert.expect(1);
var instanceNumber = 0;
var initWidget = mixins.ParentedMixin.init;
mixins.ParentedMixin.init = function () {
instanceNumber++;
return initWidget.apply(this, arguments);
};
var destroyWidget = mixins.ParentedMixin.destroy;
mixins.ParentedMixin.destroy = function () {
if (!this.isDestroyed()) {
instanceNumber--;
}
return destroyWidget.apply(this, arguments);
};
var params = {
View: FormView,
model: 'partner',
data: this.data,
arch: '<form string="Partners">' +
'<sheet>' +
'<field name="display_name"/>' +
'<field name="foo"/>' +
'<field name="bar"/>' +
'<field name="int_field"/>' +
'<field name="qux"/>' +
'<field name="trululu"/>' +
'<field name="timmy"/>' +
'<field name="product_id"/>' +
'<field name="priority"/>' +
'<field name="state"/>' +
'<field name="date"/>' +
'<field name="datetime"/>' +
'<field name="product_ids"/>' +
'<field name="p">' +
'<tree default_order="foo desc">' +
'<field name="display_name"/>' +
'<field name="foo"/>' +
'</tree>' +
'</field>' +
'</sheet>' +
'</form>',
archs: {
'partner,false,form':
'<form string="Partner">' +
'<sheet>' +
'<group>' +
'<field name="foo"/>' +
'</group>' +
'</sheet>' +
'</form>',
"partner_type,false,list": '<tree><field name="name"/></tree>',
'product,false,list': '<tree><field name="display_name"/></tree>',
},
res_id: 1,
};
var form = createView(params);
form.destroy();
var initialInstanceNumber = instanceNumber;
instanceNumber = 0;
form = createView(params);
// call destroy function of controller to ensure that it correctly destroys everything
form.__destroy();
assert.strictEqual(instanceNumber, initialInstanceNumber+1, "every widget must be destroyed exept the parent");
form.destroy();
mixins.ParentedMixin.init = initWidget;
mixins.ParentedMixin.destroy = destroyWidget;
});
});
});
@@ -5,6 +5,7 @@ var core = require('web.core');
var KanbanColumnProgressBar = require('web.KanbanColumnProgressBar');
var KanbanRenderer = require('web.KanbanRenderer');
var KanbanView = require('web.KanbanView');
var mixins = require('web.mixins');
var testUtils = require('web.test_utils');
var Widget = require('web.Widget');
var widgetRegistry = require('web.widget_registry');
@@ -2484,6 +2485,63 @@ QUnit.module('Views', {
kanban.destroy();
});
QUnit.test('check if the view destroys all widgets and instances', function (assert) {
assert.expect(1);
var instanceNumber = 0;
var initWidget = mixins.ParentedMixin.init;
mixins.ParentedMixin.init = function () {
instanceNumber++;
return initWidget.apply(this, arguments);
};
var destroyWidget = mixins.ParentedMixin.destroy;
mixins.ParentedMixin.destroy = function () {
if (!this.isDestroyed()) {
instanceNumber--;
}
return destroyWidget.apply(this, arguments);
};
var params = {
View: KanbanView,
model: 'partner',
data: this.data,
arch: '<kanban string="Partners">' +
'<field name="foo"/>' +
'<field name="bar"/>' +
'<field name="int_field"/>' +
'<field name="qux"/>' +
'<field name="product_id"/>' +
'<field name="category_ids"/>' +
'<field name="state"/>' +
'<field name="date"/>' +
'<field name="datetime"/>' +
'<templates><t t-name="kanban-box">' +
'<div><field name="foo"/></div>' +
'</t></templates>' +
'</kanban>',
};
var kanban = createView(params);
kanban.destroy();
var initialInstanceNumber = instanceNumber;
instanceNumber = 0;
kanban = createView(params);
// call destroy function of controller to ensure that it correctly destroys everything
kanban.__destroy();
assert.strictEqual(instanceNumber, initialInstanceNumber+1, "every widget must be destroyed exept the parent");
kanban.destroy();
mixins.ParentedMixin.init = initWidget;
mixins.ParentedMixin.destroy = destroyWidget;
});
});
});
@@ -5,6 +5,7 @@ var config = require('web.config');
var basicFields = require('web.basic_fields');
var FormView = require('web.FormView');
var ListView = require('web.ListView');
var mixins = require('web.mixins');
var testUtils = require('web.test_utils');
var widgetRegistry = require('web.widget_registry');
var Widget = require('web.Widget');
@@ -3246,6 +3247,62 @@ QUnit.module('Views', {
list.destroy();
});
QUnit.test('check if the view destroys all widgets and instances', function (assert) {
assert.expect(1);
var instanceNumber = 0;
var initWidget = mixins.ParentedMixin.init;
mixins.ParentedMixin.init = function () {
instanceNumber++;
return initWidget.apply(this, arguments);
};
var destroyWidget = mixins.ParentedMixin.destroy;
mixins.ParentedMixin.destroy = function () {
if (!this.isDestroyed()) {
instanceNumber--;
}
return destroyWidget.apply(this, arguments);
};
var params = {
View: ListView,
model: 'foo',
data: this.data,
arch: '<tree string="Partners">' +
'<field name="foo"/>' +
'<field name="bar"/>' +
'<field name="date"/>' +
'<field name="int_field"/>' +
'<field name="qux"/>' +
'<field name="m2o"/>' +
'<field name="o2m"/>' +
'<field name="m2m"/>' +
'<field name="amount"/>' +
'<field name="currency_id"/>' +
'<field name="datetime"/>' +
'<field name="reference"/>' +
'</tree>',
};
var list = createView(params);
list.destroy();
var initialInstanceNumber = instanceNumber;
instanceNumber = 0;
list = createView(params);
// call destroy function of controller to ensure that it correctly destroys everything
list.__destroy();
assert.strictEqual(instanceNumber, initialInstanceNumber+1, "every widget must be destroyed exept the parent");
list.destroy();
mixins.ParentedMixin.init = initWidget;
mixins.ParentedMixin.destroy = destroyWidget;
});
});
});