From 2529aa3ef2f4d09bc515cbcdc37a0c3278ec7bed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Mon, 14 Sep 2020 22:20:37 +0200
Subject: [PATCH] [FIX] component: make concurrent renderings more robust

Here is a situation that can happen in some complicated case:

1. a parent component is rendered, which includes some children
2. it is then willPatched
3. the sub components are then mounted/willUnmounted
4. because of complicated business logic, this causes the parent
component to be rerendered (before parent "patched" method is called)
5. owl will internally reset its currentfiber to null (but there is a
pending rendering!)
6. subsequent rendering will ignore pending rendering
7. havoc ensues

This is actually one of the reason why modifying a component state in a
willPatch component is actually not a good idea.  However, the good news
is that this specific situation can be properly handled: we can simply
make sure that we do not reset currentFiber to null if there is a new
pending rendering.

closes #728
---
 src/component/component.ts        |  6 +++---
 src/component/fiber.ts            |  4 +++-
 tests/component/component.test.ts | 30 ++++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/src/component/component.ts b/src/component/component.ts
index bd2bcd2a..8a7521b6 100644
--- a/src/component/component.ts
+++ b/src/component/component.ts
@@ -496,9 +496,9 @@ export class Component<Props extends {} = any, T extends Env = Env> {
     }
     this.willUnmount();
     __owl__.isMounted = false;
-    if (this.__owl__.currentFiber) {
-      this.__owl__.currentFiber.isCompleted = true;
-      this.__owl__.currentFiber.root.counter = 0;
+    if (__owl__.currentFiber) {
+      __owl__.currentFiber.isCompleted = true;
+      __owl__.currentFiber.root.counter = 0;
     }
     const children = __owl__.children;
     for (let id in children) {
diff --git a/src/component/fiber.ts b/src/component/fiber.ts
index dc6b2ba3..e981b9d8 100644
--- a/src/component/fiber.ts
+++ b/src/component/fiber.ts
@@ -249,7 +249,9 @@ export class Fiber {
           component.__owl__.pvnode!.elm = component.__owl__.vnode!.elm;
         }
       }
-      component.__owl__.currentFiber = null;
+      if (fiber === component.__owl__.currentFiber) {
+        component.__owl__.currentFiber = null;
+      }
     }
 
     // insert into the DOM (mount case)
diff --git a/tests/component/component.test.ts b/tests/component/component.test.ts
index e731c1c3..27e1ed0f 100644
--- a/tests/component/component.test.ts
+++ b/tests/component/component.test.ts
@@ -3064,6 +3064,36 @@ describe("random stuff/miscellaneous", () => {
     await nextTick();
     expect(fixture.textContent!.trim()).toBe("2__3");
   });
+
+  test("two renderings initiated between willPatch and patched", async () => {
+    let app;
+
+    class Panel extends Component {
+      static template = xml`<abc><t t-esc="props.val"/></abc>`;
+      mounted() {
+        app.render();
+      }
+      willUnmount() {
+        app.render();
+      }
+    }
+
+    // Main root component
+    class App extends Component {
+      static components = { Panel };
+      static template = xml`<div><Panel t-key="'panel_' + state.panel" val="state.panel"/></div>`;
+
+      state = useState({ panel: "Panel1" });
+    }
+
+    app = new App();
+    await app.mount(fixture);
+
+    expect(fixture.innerHTML).toBe("<div><abc>Panel1</abc></div>");
+    app.state.panel = "Panel2";
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div><abc>Panel2</abc></div>");
+  });
 });
 
 describe("widget and observable state", () => {