From 8993d6ff796ffbac056f9808557d2dbfca01b62d Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Fri, 14 Sep 2018 16:25:43 +0200
Subject: [PATCH] [FIX] web: x2Many: remove deadlock situation

This rev. is similar to db9b91ff, but treats the x2many case.
Calls to the 'commitChange' function of renderers must not be done
inside a mutex, mainly since 241704536, as they could end up in a
deadlock. For x2manys, it was actually the case in practice in
Inventory > Operations > Inventory Adjustments > Create > select
'One product only' > select a product > try to edit the real
quantity in the editable list > focusout > deadlock.

The reason why the deadlock occurred in this view and not in, e.g.
sale.order, is due to the presence of the field_float_scannable
widget. Indeed, the barcode engine intercepts input events and
manually re-triggers them. As a consequence, the 'change' event is
not triggered on the field, and it does not notify the environment
of its new value by itself, until the field x2many asks it to
commit its value. Note that this might be an issue as well, as
onchanges are thus not triggered.

Task 1878254
---
 .../static/src/js/fields/relational_fields.js | 20 +++-----
 .../tests/fields/relational_fields_tests.js   | 50 +++++++++++++++++++
 2 files changed, 58 insertions(+), 12 deletions(-)

diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js
index 4c92c0973b9..a1efa85498e 100644
--- a/addons/web/static/src/js/fields/relational_fields.js
+++ b/addons/web/static/src/js/fields/relational_fields.js
@@ -1221,18 +1221,14 @@ var FieldX2Many = AbstractField.extend({
     _onSaveLine: function (ev) {
         var self = this;
         ev.stopPropagation();
-        this.trigger_up('mutexify', {
-            action: function () {
-                return self.renderer.commitChanges(ev.data.recordID).then(function () {
-                    self.trigger_up('mutexify', {
-                        action: function () {
-                            return self._saveLine(ev.data.recordID)
-                                .done(ev.data.onSuccess)
-                                .fail(ev.data.onFailure);
-                        },
-                    });
-                });
-            },
+        this.renderer.commitChanges(ev.data.recordID).then(function () {
+            self.trigger_up('mutexify', {
+                action: function () {
+                    return self._saveLine(ev.data.recordID)
+                        .done(ev.data.onSuccess)
+                        .fail(ev.data.onFailure);
+                },
+            });
         });
     },
     /**
diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js
index 2fff96e8ee3..8809d219e49 100644
--- a/addons/web/static/tests/fields/relational_fields_tests.js
+++ b/addons/web/static/tests/fields/relational_fields_tests.js
@@ -10331,6 +10331,56 @@ QUnit.module('relational_fields', {
         form.destroy();
     });
 
+    QUnit.test('no deadlock when leaving a one2many line with uncommitted changes', function (assert) {
+        // Before unselecting a o2m line, field widgets are asked to commit their changes (new values
+        // that they wouldn't have sent to the model yet). This test is added alongside a bug fix
+        // ensuring that we don't end up in a deadlock when a widget actually has some changes to
+        // commit at that moment.
+        assert.expect(9);
+         var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form>' +
+                    '<field name="turtles">' +
+                        '<tree editable="bottom">' +
+                            '<field name="turtle_foo"/>' +
+                        '</tree>' +
+                    '</field>' +
+                '</form>',
+            mockRPC: function (route, args) {
+                assert.step(args.method);
+                return this._super.apply(this, arguments);
+            },
+             // we set a fieldDebounce to precisely mock the behavior of the webclient: changes are
+             // not sent to the model at keystrokes, but when the input is left
+            fieldDebounce: 5000,
+        });
+
+        form.$('.o_field_x2many_list_row_add a').click();
+
+        form.$('input').val('some foo value').trigger('input');
+
+        // click to add a second row to unselect the current one, then save
+        form.$('.o_field_x2many_list_row_add a').click();
+        form.$buttons.find('.o_form_button_save').click();
+
+        assert.strictEqual(form.$('.o_form_readonly').length, 1,
+            "form view should be in readonly");
+        assert.strictEqual(form.$('.o_data_row').text(), 'some foo value',
+            "foo field should have correct value");
+        assert.verifySteps([
+            'default_get', // main record
+            'default_get', // line 1
+            'default_get', // line 2
+            'create',
+            'read', // main record
+            'read', // line 1
+        ]);
+
+        form.destroy();
+    });
+
     QUnit.module('FieldMany2Many');
 
     QUnit.test('many2many kanban: edition', function (assert) {