From 840f914a4ee5a9bfd0ac4f4565d158d2c253cd94 Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Thu, 10 Jun 2021 07:50:58 +0000
Subject: [PATCH] [FIX] web: concurrency issue when opening form view
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

From a list or kanban view, quickly click multiple times on a
record or on the Create button: a lot of bad stuff could happen
from there (e.g. crashes, form view that could not be open
anymore afterward).

The reason was that we destroyed the legacy form view whereas we
shouldn't, as it was still used. In more details, when we clicked
first on a record to open it, we created an owl Component to
instantiate and wrap the form view (legacy). When we clicked a
second time, we another owl Component was instantiated and the
first one was destroyed. As an unwanted side-effect, the legacy
form view was destroyed as well, but the new controller wanted to
reuse it. From that point, anything bad could happen as we were
trying to use a destroyed widget.

closes odoo-dev/odoo#941

Signed-off-by: Géry Debongnie (ged) <ged@openerp.com>
---
 .../web/static/src/legacy/action_adapters.js  | 22 +++++++++++-
 .../src/webclient/actions/action_service.js   |  3 ++
 .../webclient/actions/concurrency_tests.js    | 35 +++++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/addons/web/static/src/legacy/action_adapters.js b/addons/web/static/src/legacy/action_adapters.js
index 95e4f773726..d0c234ed4a1 100644
--- a/addons/web/static/src/legacy/action_adapters.js
+++ b/addons/web/static/src/legacy/action_adapters.js
@@ -69,6 +69,10 @@ class ActionAdapter extends ComponentAdapter {
         );
     }
 
+    get actionId() {
+        throw new Error("Should be implement by specific adapters");
+    }
+
     _trigger_up(ev) {
         const payload = ev.data;
         if (ev.name === "do_action") {
@@ -147,6 +151,12 @@ class ActionAdapter extends ComponentAdapter {
         }
         super.willUnmount();
     }
+    __destroy() {
+        if (this.actionService.__legacy__isActionInStack(this.actionId)) {
+            this.widget = null;
+        }
+        super.__destroy(...arguments);
+    }
 }
 
 export class ClientActionAdapter extends ActionAdapter {
@@ -156,6 +166,10 @@ export class ClientActionAdapter extends ActionAdapter {
         this.env = Component.env;
     }
 
+    get actionId() {
+        return this.props.widgetArgs[0].jsId;
+    }
+
     async willStart() {
         if (this.props.widget) {
             this.widget = this.props.widget;
@@ -240,6 +254,10 @@ export class ViewAdapter extends ActionAdapter {
         this.env = Component.env;
     }
 
+    get actionId() {
+        return this.props.viewParams.action.jsId;
+    }
+
     async willStart() {
         if (this.props.widget) {
             this.widget = this.props.widget;
@@ -254,7 +272,9 @@ export class ViewAdapter extends ActionAdapter {
             if (this.__owl__.status === 5 /* DESTROYED */) {
                 // the component might have been destroyed meanwhile, but if so, `this.widget` wasn't
                 // destroyed by OwlCompatibility layer as it wasn't set yet, so destroy it now
-                this.widget.destroy();
+                if (!this.actionService.__legacy__isActionInStack(this.actionId)) {
+                    this.widget.destroy();
+                }
                 return Promise.resolve();
             }
             return this.widget._widgetRenderAndInsert(() => {});
diff --git a/addons/web/static/src/webclient/actions/action_service.js b/addons/web/static/src/webclient/actions/action_service.js
index bb051382a39..23b24263389 100644
--- a/addons/web/static/src/webclient/actions/action_service.js
+++ b/addons/web/static/src/webclient/actions/action_service.js
@@ -1215,6 +1215,9 @@ function makeActionManager(env) {
         get currentController() {
             return _getCurrentController();
         },
+        __legacy__isActionInStack(actionId) {
+            return controllerStack.find((c) => c.action.jsId === actionId);
+        },
     };
 }
 
diff --git a/addons/web/static/tests/webclient/actions/concurrency_tests.js b/addons/web/static/tests/webclient/actions/concurrency_tests.js
index c879df5a97e..e68b66cbae6 100644
--- a/addons/web/static/tests/webclient/actions/concurrency_tests.js
+++ b/addons/web/static/tests/webclient/actions/concurrency_tests.js
@@ -582,4 +582,39 @@ QUnit.module("ActionManager", (hooks) => {
             "/web/dataset/search_read",
         ]);
     });
+
+    QUnit.test("click multiple times to open a record", async function (assert) {
+        assert.expect(5);
+
+        const def = testUtils.makeTestPromise();
+        const defs = [null, def];
+        const mockRPC = async (route, args) => {
+            if (args.method === "read") {
+                await Promise.resolve(defs.shift());
+            }
+        };
+
+        const webClient = await createWebClient({ serverData, mockRPC });
+        await doAction(webClient, 3);
+        assert.containsOnce(webClient, ".o_list_view");
+
+        await testUtils.dom.click(webClient.el.querySelector(".o_list_view .o_data_row"));
+        await legacyExtraNextTick();
+        assert.containsOnce(webClient, ".o_form_view");
+
+        await testUtils.dom.click(webClient.el.querySelector(".o_back_button"));
+        await legacyExtraNextTick();
+
+        assert.containsOnce(webClient, ".o_list_view");
+
+        await testUtils.dom.click(webClient.el.querySelector(".o_list_view .o_data_row"));
+        await testUtils.dom.click(webClient.el.querySelector(".o_list_view .o_data_row"));
+        await legacyExtraNextTick();
+        assert.containsOnce(webClient, ".o_list_view");
+
+        def.resolve();
+        await nextTick();
+        await legacyExtraNextTick();
+        assert.containsOnce(webClient, ".o_form_view");
+    });
 });