From 71a2d7c06a6142bfe3e967aa8ff60552e24eb0c1 Mon Sep 17 00:00:00 2001
From: Pierre Paridans <app@odoo.com>
Date: Wed, 27 Mar 2019 13:50:22 +0000
Subject: [PATCH] [REF] web: don't destroy twice a widget's children
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Widget class indirectly inherits from ParentedMixin (through
PropertiesMixin).

The Widget's children are destroyed in both the ParentedMixin and the
Widget's own destroy() method.

This commit prevents from trying to destroy already destroyed children
and specifically from looping again on those children (resulting into a
minor performance improvement).

closes odoo/odoo#32174

Signed-off-by: Adrien Dieudonné (adr) <adr@odoo.com>
---
 addons/web/static/src/js/core/mixins.js      |  4 ++--
 addons/web/static/src/js/core/widget.js      |  5 +----
 addons/web/static/tests/core/widget_tests.js | 14 ++++++++++++++
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/addons/web/static/src/js/core/mixins.js b/addons/web/static/src/js/core/mixins.js
index 228ac2d68dd..06f39e0fcfe 100644
--- a/addons/web/static/src/js/core/mixins.js
+++ b/addons/web/static/src/js/core/mixins.js
@@ -98,8 +98,8 @@ var ParentedMixin = {
      * resource it could have reserved.
      */
     destroy : function () {
-        _.each(this.getChildren(), function (el) {
-            el.destroy();
+        this.getChildren().forEach(function (child) {
+            child.destroy();
         });
         this.setParent(undefined);
         this.__parentedDestroyed = true;
diff --git a/addons/web/static/src/js/core/widget.js b/addons/web/static/src/js/core/widget.js
index 0de8c6aa1cd..441899e30db 100644
--- a/addons/web/static/src/js/core/widget.js
+++ b/addons/web/static/src/js/core/widget.js
@@ -172,13 +172,10 @@ var Widget = core.Class.extend(mixins.PropertiesMixin, ServicesMixin, {
      * destroying itself.
      */
     destroy: function () {
-        this.getChildren().forEach(function (child) {
-            child.destroy();
-        });
+        mixins.PropertiesMixin.destroy.call(this);
         if (this.$el) {
             this.$el.remove();
         }
-        mixins.PropertiesMixin.destroy.call(this);
     },
 
     //--------------------------------------------------------------------------
diff --git a/addons/web/static/tests/core/widget_tests.js b/addons/web/static/tests/core/widget_tests.js
index 440cc97b524..13515e08641 100644
--- a/addons/web/static/tests/core/widget_tests.js
+++ b/addons/web/static/tests/core/widget_tests.js
@@ -468,6 +468,20 @@ QUnit.module('core', {}, function () {
         slowWillStartPromise.resolve();
     });
 
+    QUnit.test("don't destroy twice widget's children", function (assert) {
+        assert.expect(2);
+
+        var parent = new Widget();
+        var child = new (Widget.extend({
+            destroy: function () {
+                assert.step('destroy');
+            }
+        }))(parent);
+
+        parent.destroy();
+        assert.verifySteps(['destroy'], "child should have been detroyed only once");
+    });
+
 });
 
 });