[FIX] web: basic_controller: wait for the mutex before discard changes

Before this commit, the discard confirm dialog was directly shown
without waiting for the write RPC. For instance, it the user clicked on 'Save'
after editing a record, and then directly clicked on another menu or on the
breadcrumb before the write RPC returned, the record was still considered
as dirty and thus the confirm dialog was displayed. We now wait for the RPC
to return before checking if the record is dirty, so that the confirm dialog
is only displayed when the record is actually dirty.

3 files changed, 67 insertions, 57 deletions

diff --git a/addons/web/static/src/js/views/basic/basic_controller.js b/addons/web/static/src/js/views/basic/basic_controller.js
index 67a4fa504d8..82dc6f90d26 100644
--- a/addons/web/static/src/js/views/basic/basic_controller.js
+++ b/addons/web/static/src/js/views/basic/basic_controller.js
@@ -104,39 +104,15 @@ var BasicController = AbstractController.extend(FieldManagerMixin, {
         return true;
     },
     /**
-     * Discards the changes made to the record whose ID is given, if necessary.
-     * Automatically leaves to default mode for the given record.
+     * Waits for the mutex to be unlocked and then calls _.discardChanges.
+     * This ensures that the confirm dialog isn't displayed directly if there is
+     * a pending 'write' rpc.
      *
-     * @param {string} [recordID] - default to main recordID
-     * @param {Object} [options]
-     * @param {boolean} [options.readonlyIfRealDiscard=false]
-     *        After discarding record changes, the usual option is to make the
-     *        record readonly. However, the view manager calls this function
-     *        at inappropriate times in the current code and in that case, we
-     *        don't want to go back to readonly if there is nothing to discard
-     *        (e.g. when switching record in edit mode in form view, we expect
-     *        the new record to be in edit mode too, but the view manager calls
-     *        this function as the URL changes...) @todo get rid of this when
-     *        the view manager is improved.
-     * @returns {Deferred}
+     * @see _.discardChanges
      */
     discardChanges: function (recordID, options) {
-        var self = this;
-        recordID = recordID || this.handle;
-        return this.canBeDiscarded(recordID).then(function (needDiscard) {
-            if (options && options.readonlyIfRealDiscard && !needDiscard) {
-                return;
-            }
-
-            if (needDiscard) { // Just some optimization
-                self.model.discardChanges(recordID);
-            }
-            if (self.model.isNew(recordID)) {
-                self._abandonRecord(recordID);
-                return;
-            }
-            return self._confirmSave(recordID);
-        });
+        return this.mutex.exec(function () {})
+            .then(this._discardChanges.bind(this, recordID || this.handle, options));
     },
     /**
      * Method that will be overriden by the views with the ability to have selected ids
@@ -327,6 +303,42 @@ var BasicController = AbstractController.extend(FieldManagerMixin, {
         }
     },
     /**
+     * Discards the changes made to the record whose ID is given, if necessary.
+     * Automatically leaves to default mode for the given record.
+     *
+     * @private
+     * @param {string} [recordID] - default to main recordID
+     * @param {Object} [options]
+     * @param {boolean} [options.readonlyIfRealDiscard=false]
+     *        After discarding record changes, the usual option is to make the
+     *        record readonly. However, the view manager calls this function
+     *        at inappropriate times in the current code and in that case, we
+     *        don't want to go back to readonly if there is nothing to discard
+     *        (e.g. when switching record in edit mode in form view, we expect
+     *        the new record to be in edit mode too, but the view manager calls
+     *        this function as the URL changes...) @todo get rid of this when
+     *        the view manager is improved.
+     * @returns {Deferred}
+     */
+    _discardChanges: function (recordID, options) {
+        var self = this;
+        recordID = recordID || this.handle;
+        return this.canBeDiscarded(recordID)
+            .then(function (needDiscard) {
+                if (options && options.readonlyIfRealDiscard && !needDiscard) {
+                    return;
+                }
+                if (needDiscard) { // Just some optimization
+                    self.model.discardChanges(recordID);
+                }
+                if (self.model.isNew(recordID)) {
+                    self._abandonRecord(recordID);
+                    return;
+                }
+                return self._confirmSave(recordID);
+            });
+    },
+    /**
      * Enables buttons so they can be clicked again.
      *
      * @private
@@ -501,11 +513,8 @@ var BasicController = AbstractController.extend(FieldManagerMixin, {
         var self = this;
         ev.stopPropagation();
         var recordID = ev.data.recordID;
-        this.discardChanges(recordID)
+        this._discardChanges(recordID)
             .done(function () {
-                if (self.model.isNew(recordID)) {
-                    self._abandonRecord(recordID);
-                }
                 // TODO this will tell the renderer to rerender the widget that
                 // asked for the discard but will unfortunately lose the click
                 // made on another row if any
diff --git a/addons/web/static/src/js/views/form/form_controller.js b/addons/web/static/src/js/views/form/form_controller.js
index 8a42168a098..2b607496e85 100644
--- a/addons/web/static/src/js/views/form/form_controller.js
+++ b/addons/web/static/src/js/views/form/form_controller.js
@@ -409,7 +409,7 @@ var FormController = BasicController.extend({
      * @private
      */
     _onDiscard: function () {
-        this.discardChanges();
+        this._discardChanges();
     },
     /**
      * Called when the user clicks on 'Duplicate Record' in the sidebar
diff --git a/addons/web/static/src/js/views/list/list_controller.js b/addons/web/static/src/js/views/list/list_controller.js
index ee0fd323ce1..2ec9cf3181e 100644
--- a/addons/web/static/src/js/views/list/list_controller.js
+++ b/addons/web/static/src/js/views/list/list_controller.js
@@ -50,27 +50,6 @@ var ListController = BasicController.extend({
     //--------------------------------------------------------------------------
 
     /**
-     * To improve performance, list view must not be rerendered if it is asked
-     * to discard all its changes. Indeed, only the in-edition row needs to be
-     * discarded in that case.
-     *
-     * @override
-     * @param {string} [recordID] - default to main recordID
-     * @returns {Deferred}
-     */
-    discardChanges: function (recordID) {
-        if ((recordID || this.handle) === this.handle) {
-            recordID = this.renderer.getEditableRecordID();
-            if (recordID === null) {
-                return $.when();
-            }
-        }
-        var self = this;
-        return this._super(recordID).then(function () {
-            self._updateButtons('readonly');
-        });
-    },
-    /**
      * Calculate the active domain of the list view. This should be done only
      * if the header checkbox has been checked. This is done by evaluating the
      * search results, and then adding the dataset domain (i.e. action domain).
@@ -246,6 +225,28 @@ var ListController = BasicController.extend({
             .then(this._setMode.bind(this, 'readonly', id));
     },
     /**
+     * To improve performance, list view must not be rerendered if it is asked
+     * to discard all its changes. Indeed, only the in-edition row needs to be
+     * discarded in that case.
+     *
+     * @override
+     * @private
+     * @param {string} [recordID] - default to main recordID
+     * @returns {Deferred}
+     */
+    _discardChanges: function (recordID) {
+        if ((recordID || this.handle) === this.handle) {
+            recordID = this.renderer.getEditableRecordID();
+            if (recordID === null) {
+                return $.when();
+            }
+        }
+        var self = this;
+        return this._super(recordID).then(function () {
+            self._updateButtons('readonly');
+        });
+    },
+    /**
      * @override
      * @private
      */
@@ -362,7 +363,7 @@ var ListController = BasicController.extend({
      */
     _onDiscard: function (ev) {
         ev.stopPropagation(); // So that it is not considered as a row leaving
-        this.discardChanges();
+        this._discardChanges();
     },
     /**
      * Called when the user asks to edit a row -> Updates the controller buttons