[FIX] web: ActionManager: concurrent window actions

This rev. strengthens the handling of concurrent actions of type
'ir.actions.act_window' in the ActionManager. In particular, the
two following cases are now handled:

 - execute an action A, its 'load_views' RPC takes a while, during
   that time, execute an action B: A can be dropped, meaning that
   it doesn't need to fetch its data, and thus it should not be
   appended to the DOM (nor appear in the breadcrumbs) ;
 - execute an action A, its 'read' or 'search_read' RPC takes a
   while, during that time, execute an action B: A can be dropped
   and should not be appended to the DOM (nor appear in the
   breadcrumbs).

addons/web/static/src/js/chrome/action_manager.js

@@ -61,7 +61,6 @@ var ActionManager = Widget.extend({
         // dialog (i.e. coming from an action with target='new')
         this.currentDialogController = null;
     },
-
     /**
      * @override
      */
@@ -144,7 +143,7 @@ var ActionManager = Widget.extend({
      *   loaded and appended to the DOM ; rejected if the action can't be
      *   executed (e.g. if doAction has been called to execute another action
      *   before this one was complete).
-    */
+     */
     doAction: function (action, options) {
         var self = this;
         options = _.defaults({}, options, {
@@ -907,13 +906,13 @@ var ActionManager = Widget.extend({
         }
     },
     /**
-    * Intercepts and triggers a redirection on a link
-    *
-    * @private
-    * @param {OdooEvent} ev
-    * @param {integer} ev.data.res_id
-    * @param {string} ev.data.res_model
-    */
+     * Intercepts and triggers a redirection on a link.
+     *
+     * @private
+     * @param {OdooEvent} ev
+     * @param {integer} ev.data.res_id
+     * @param {string} ev.data.res_model
+     */
     _onRedirect: function (ev) {
         this.do_action({
             type:'ir.actions.act_window',

addons/web/static/src/js/chrome/action_manager_act_window.js

@@ -223,6 +223,31 @@ ActionManager.include({
 
         return action.controllers[viewType];
     },
+    /**
+     * Destroys the controllers and search view of a given action of type
+     * 'ir.actions.act_window'.
+     *
+     * @private
+     * @param {Object} action
+     */
+    _destroyWindowAction: function (action) {
+        var self = this;
+        _.each(action.controllers, function (controllerDef) {
+            controllerDef.then(function (controller) {
+                delete self.controllers[controller.jsID];
+                if (controller.widget) {
+                    controller.widget.destroy();
+                }
+            });
+            // reject the deferred if it is not yet resolved, so that the
+            // controller is correctly destroyed as soon as it is ready, and
+            // its reference is removed
+            controllerDef.reject();
+        });
+        if (action.searchView) {
+            action.searchView.destroy();
+        }
+    },
     /**
      * Executes actions of type 'ir.actions.act_window'.
      *
@@ -245,7 +270,7 @@ ActionManager.include({
         }
         action.flags = this._generateActionFlags(action);
 
-        return this._loadViews(action).then(function (fieldsViews) {
+        return this.dp.add(this._loadViews(action)).then(function (fieldsViews) {
             var views = self._generateActionViews(action, fieldsViews);
             action._views = action.views;  // save the initial attribute
             action.views = views;
@@ -291,7 +316,7 @@ ActionManager.include({
                 if (mainView) {
                     defs.push(self._createViewController(action, mainView.type, {}, {lazy: true}));
                 }
-                return $.when.apply($, defs);
+                return self.dp.add($.when.apply($, defs));
             }).then(function (controller, lazyLoadedController) {
                 action.controllerID = controller.jsID;
                 return self._executeAction(action, options).done(function () {
@@ -302,7 +327,7 @@ ActionManager.include({
                         }, {clear: false});
                     }
                 });
-            });
+            }).fail(self._destroyWindowAction.bind(self, action));
         });
     },
     /**
@@ -490,25 +515,10 @@ ActionManager.include({
      * @private
      */
     _removeAction: function (actionID) {
-        var self = this;
         var action = this.actions[actionID];
         if (action.type === 'ir.actions.act_window') {
             delete this.actions[action.jsID];
-            _.each(action.controllers, function (controllerDef) {
-                controllerDef.then(function (controller) {
-                    delete self.controllers[controller.jsID];
-                    if (controller.widget) {
-                        controller.widget.destroy();
-                    }
-                });
-                // reject the deferred if it is not yet resolved, so that the
-                // controller is correctly destroyed as soon as it is ready, and
-                // its reference is removed
-                controllerDef.reject();
-            });
-            if (action.searchView) {
-                action.searchView.destroy();
-            }
+            this._destroyWindowAction(action);
         } else {
             this._super.apply(this, arguments);
         }

addons/web/static/tests/chrome/action_manager_tests.js

@@ -276,6 +276,102 @@ QUnit.module('ActionManager', {
         testUtils.unpatch(Widget);
     });
 
+    QUnit.test('no memory leaks when executing an action while loading views', function (assert) {
+        assert.expect(1);
+
+        var def;
+        var delta = 0;
+        testUtils.patch(Widget, {
+            init: function () {
+                delta += 1;
+                this._super.apply(this, arguments);
+            },
+            destroy: function () {
+                delta -= 1;
+                this._super.apply(this, arguments);
+            },
+        });
+
+        var actionManager = createActionManager({
+            actions: this.actions,
+            archs: this.archs,
+            data: this.data,
+            mockRPC: function (route, args) {
+               var result = this._super.apply(this, arguments);
+                if (args.method === 'load_views') {
+                    return $.when(def).then(_.constant(result));
+                }
+                return result;
+            },
+        });
+
+        // execute action 4 to know the number of widgets it instantiates
+        actionManager.doAction(4);
+        var n = delta;
+
+        // execute a first action (its 'load_views' RPC is blocked)
+        def = $.Deferred();
+        actionManager.doAction(3, {clear_breadcrumbs: true});
+
+        // execute another action meanwhile (and unlock the RPC)
+        actionManager.doAction(4, {clear_breadcrumbs: true});
+        def.resolve();
+
+        assert.strictEqual(n, delta,
+            "all widgets of action 3 should have been destroyed");
+
+        actionManager.destroy();
+        testUtils.unpatch(Widget);
+    });
+
+    QUnit.test('no memory leaks when executing an action while loading data of default view', function (assert) {
+        assert.expect(1);
+
+        var def;
+        var delta = 0;
+        testUtils.patch(Widget, {
+            init: function () {
+                delta += 1;
+                this._super.apply(this, arguments);
+            },
+            destroy: function () {
+                delta -= 1;
+                this._super.apply(this, arguments);
+            },
+        });
+
+        var actionManager = createActionManager({
+            actions: this.actions,
+            archs: this.archs,
+            data: this.data,
+            mockRPC: function (route) {
+                var result = this._super.apply(this, arguments);
+                if (route === '/web/dataset/search_read') {
+                    return $.when(def).then(_.constant(result));
+                }
+                return result;
+            },
+        });
+
+        // execute action 4 to know the number of widgets it instantiates
+        actionManager.doAction(4);
+        var n = delta;
+
+        // execute a first action (its 'search_read' RPC is blocked)
+        def = $.Deferred();
+        actionManager.doAction(3, {clear_breadcrumbs: true});
+
+        // execute another action meanwhile (and unlock the RPC)
+        actionManager.doAction(4, {clear_breadcrumbs: true});
+        def.resolve();
+
+        assert.strictEqual(n, delta,
+            "all widgets of action 3 should have been destroyed");
+
+        actionManager.destroy();
+        testUtils.unpatch(Widget);
+    });
+
     QUnit.test('action with "no_breadcrumbs" set to true', function (assert) {
         assert.expect(2);
 
@@ -1256,6 +1352,101 @@ QUnit.module('ActionManager', {
         actionManager.destroy();
     });
 
+    QUnit.test('execute a new action while loading views', function (assert) {
+        assert.expect(10);
+
+        var def;
+        var actionManager = createActionManager({
+            actions: this.actions,
+            archs: this.archs,
+            data: this.data,
+            mockRPC: function (route, args) {
+                var result = this._super.apply(this, arguments);
+                assert.step(args.method || route);
+                if (args.method === 'load_views') {
+                    return $.when(def).then(_.constant(result));
+                }
+                return result;
+            },
+        });
+
+        // execute a first action (its 'load_views' RPC is blocked)
+        def = $.Deferred();
+        actionManager.doAction(3);
+
+        assert.strictEqual(actionManager.$('.o_list_view').length, 0,
+            "should not display the list view of action 3");
+
+        // execute another action meanwhile (and unlock the RPC)
+        actionManager.doAction(4);
+        def.resolve();
+
+        assert.strictEqual(actionManager.$('.o_kanban_view').length, 1,
+            "should display the kanban view of action 4");
+        assert.strictEqual(actionManager.$('.o_list_view').length, 0,
+            "should not display the list view of action 3");
+        assert.strictEqual($('.o_control_panel .breadcrumb li').length, 1,
+            "there should be one controller in the breadcrumbs");
+
+        assert.verifySteps([
+            '/web/action/load', // action 3
+            'load_views', // action 3
+            '/web/action/load', // action 4
+            'load_views', // action 4
+            '/web/dataset/search_read', // search read action 4
+        ]);
+
+        actionManager.destroy();
+    });
+
+    QUnit.test('execute a new action while loading data of default view', function (assert) {
+        assert.expect(11);
+
+        var def;
+        var actionManager = createActionManager({
+            actions: this.actions,
+            archs: this.archs,
+            data: this.data,
+            mockRPC: function (route, args) {
+                var result = this._super.apply(this, arguments);
+                assert.step(args.method || route);
+                if (route === '/web/dataset/search_read') {
+                    return $.when(def).then(_.constant(result));
+                }
+                return result;
+            },
+        });
+
+        // execute a first action (its 'search_read' RPC is blocked)
+        def = $.Deferred();
+        actionManager.doAction(3);
+
+        assert.strictEqual(actionManager.$('.o_list_view').length, 0,
+            "should not display the list view of action 3");
+
+        // execute another action meanwhile (and unlock the RPC)
+        actionManager.doAction(4);
+        def.resolve();
+
+        assert.strictEqual(actionManager.$('.o_kanban_view').length, 1,
+            "should display the kanban view of action 4");
+        assert.strictEqual(actionManager.$('.o_list_view').length, 0,
+            "should not display the list view of action 3");
+        assert.strictEqual($('.o_control_panel .breadcrumb li').length, 1,
+            "there should be one controller in the breadcrumbs");
+
+        assert.verifySteps([
+            '/web/action/load', // action 3
+            'load_views', // action 3
+            '/web/dataset/search_read', // search read action 3
+            '/web/action/load', // action 4
+            'load_views', // action 4
+            '/web/dataset/search_read', // search read action 4
+        ]);
+
+        actionManager.destroy();
+    });
+
     QUnit.module('Client Actions');
 
     QUnit.test('can execute client actions from tag name', function (assert) {