From 47cf31b539ade1e71450184300a338acb0cab9fa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Thu, 19 Apr 2018 17:04:30 +0200
Subject: [PATCH] [FIX] web: prevent web client from blocking some view

In some situations, it could happen that the action manager had a
rejected deferred representing the creation of a view, and was blocked
from doing any other interaction with that view.

For example, imagine that a read operation crashes for record 1 on the
model res.partner.  If the user is on the kanban view and click on
record 1, there will be an error dialog informing him of the error.
However, if the user clicks then on another record, nothing will happen
because the action manager is blocked (only the form view in this case).

With this commit, whenever the view creation process failed, we also
remove the rejected deferred so the action manager will be allowed to
try again.
---
 .../js/chrome/action_manager_act_window.js    |  9 ++++-
 .../tests/chrome/action_manager_tests.js      | 39 +++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/addons/web/static/src/js/chrome/action_manager_act_window.js b/addons/web/static/src/js/chrome/action_manager_act_window.js
index 2621071731c..39277248480 100644
--- a/addons/web/static/src/js/chrome/action_manager_act_window.js
+++ b/addons/web/static/src/js/chrome/action_manager_act_window.js
@@ -574,7 +574,14 @@ ActionManager.include({
         };
 
         var controllerDef = action.controllers[viewType];
-        if (!controllerDef) {
+        if (!controllerDef || controllerDef.state() === 'rejected') {
+            // if the controllerDef is rejected, it probably means that the js
+            // code or the requests made to the server crashed.  In that case,
+            // if we reuse the same deferred, then the switch to the view is
+            // definitely blocked.  We want to use a new controller, even though
+            // it is very likely that it will recrash again.  At least, it will
+            // give more feedback to the user, and it could happen that one
+            // record crashes, but not another.
             controllerDef = newController();
         } else {
             controllerDef = controllerDef.then(function (controller) {
diff --git a/addons/web/static/tests/chrome/action_manager_tests.js b/addons/web/static/tests/chrome/action_manager_tests.js
index e6f97c20bdd..d987884ff83 100644
--- a/addons/web/static/tests/chrome/action_manager_tests.js
+++ b/addons/web/static/tests/chrome/action_manager_tests.js
@@ -2947,6 +2947,45 @@ QUnit.module('ActionManager', {
         delete core.action_registry.map.slowAction;
     });
 
+    QUnit.test('web client is not deadlocked when a view crashes', function (assert) {
+        assert.expect(3);
+
+        var readOnFirstRecordDef = $.Deferred();
+
+        var actionManager = createActionManager({
+            actions: this.actions,
+            archs: this.archs,
+            data: this.data,
+            mockRPC: function (route, args) {
+                if (args.method === 'read' && args.args[0][0] === 1) {
+                    return readOnFirstRecordDef;
+                }
+                return this._super.apply(this, arguments);
+            }
+        });
+
+        actionManager.doAction(3);
+
+        // open first record in form view. this will crash and will not
+        // display a form view
+        actionManager.$('.o_list_view .o_data_row:first').click();
+
+        readOnFirstRecordDef.reject("not working as intended");
+
+        assert.strictEqual(actionManager.$('.o_list_view').length, 1,
+            "there should still be a list view in dom");
+
+        // open another record, the read will not crash
+        actionManager.$('.o_list_view .o_data_row:eq(2)').click();
+
+        assert.strictEqual(actionManager.$('.o_list_view').length, 0,
+            "there should not be a list view in dom");
+
+        assert.strictEqual(actionManager.$('.o_form_view').length, 1,
+            "there should be a form view in dom");
+
+        actionManager.destroy();
+    });
 });
 
 });