From 3d0b77a711e7d4fbb4710ff981fefd76dc058323 Mon Sep 17 00:00:00 2001
From: "Simon Genin (ges)" <ges@odoo.com>
Date: Fri, 26 Mar 2021 10:52:34 +0000
Subject: [PATCH] [FIX] web: remove memory leak in editable list used as
 one2many

The confirmUpdate method in list_editable_renderer would destroy all
rows' widgets and recreate them *except* the currently modified one
(this one gets updated).

The problem was that the widgets of the current row were recreated
anyway. It created a memory leak.

This memory leak isn't such a big deal, as anything is garbage
collected as soon as the view is left anyway (so it's a small leak
during the lifetime of the x2many list)

The fix consists in keeping the reference of the widgets on the
currently modified row, and when all rows' widgets are recreated, we
delete a replace by our reference for the current row.

closes odoo/odoo#68386

Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
---
 .../js/views/list/list_editable_renderer.js   | 11 +++++
 .../relational_fields/field_one2many_tests.js | 47 +++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/addons/web/static/src/js/views/list/list_editable_renderer.js b/addons/web/static/src/js/views/list/list_editable_renderer.js
index dc2281e046e..230733448e2 100644
--- a/addons/web/static/src/js/views/list/list_editable_renderer.js
+++ b/addons/web/static/src/js/views/list/list_editable_renderer.js
@@ -230,6 +230,11 @@ ListRenderer.include({
                 }
             });
 
+             // Keep ref of the current row's widgets 
+             const currentRowFieldWidgets = self.allFieldWidgets[id];
+             // Remove it from the list just for a clean start for generating the new content (avoid later memory leak)
+             delete self.allFieldWidgets[id];
+
             // re-render whole body (outside the dom)
             self.defs = [];
             var $newBody = self._renderBody();
@@ -237,6 +242,12 @@ ListRenderer.include({
             delete self.defs;
 
             return Promise.all(defs).then(function () {
+                // All the field widgets have now be recreated, but we don't want the current row's ones.
+                // We destroy it and get it back from our reference. 
+                // This manipulation avoids a memory leak, where field widgets in the current row wouldn't be destroyed and were kept in memory.
+                self._destroyFieldWidgets(id);
+                self.allFieldWidgets[id] = currentRowFieldWidgets;
+
                 // update registered modifiers to edit 'mode' because the call to
                 // _renderBody set baseModeByRecord as 'readonly'
                 _.each(self.columns, function (node) {
diff --git a/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js b/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js
index ca4dd33c7a2..ab72d7d5764 100644
--- a/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js
+++ b/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js
@@ -9868,8 +9868,55 @@ QUnit.module('fields', {}, function () {
 
             assert.hasClass(form.$('.o_data_row:nth(1) .o_field_badge'), 'bg-warning-light');
 
+            form.destroy();
+        });
+
+        QUnit.test('Editable list renderer confirmUpdate method does not create a memory leak by no deleted currently modified row widgets but recreating them anyway.', async function (assert) {
+            assert.expect(5);
+
+            let count = 0;
+            const MyField = AbstractField.extend({
+                init() {
+                    this._super(...arguments);
+                    count++;
+                },
+                destroy() {
+                    this._super(...arguments);
+                    count--;
+                }
+            });
+            fieldRegistry.add('myfield', MyField);
+
+            this.data.partner.records[0].p = [1, 2];
+            const form = await createView({
+                View: FormView,
+                model: 'partner',
+                data: this.data,
+                arch: `
+                    <form>
+                        <field name="p">
+                            <tree editable="bottom">
+                                <field name="int_field"/>
+                                <field name="foo" widget="myfield"/>
+                            </tree>
+                        </field>
+                    </form>`,
+                res_id: 1,
+            });
+
+            assert.containsN(form, '.o_data_row', 2);
+            assert.strictEqual(count, 2);
+
+            await testUtils.dom.click(form.$('.o_data_row .o_data_cell:first'));
+            assert.strictEqual(count, 2);
+
+            await testUtils.fields.editInput(form.$('.o_selected_row .o_field_integer'), '44');
+            assert.strictEqual(count, 2);
+
             form.destroy();
             delete fieldRegistry.map.my_field;
+
+            assert.strictEqual(count, 0);
         });
 
     });