From 690d8edf67ab2d1394fc1c295f9c1413271a7bf5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Fri, 25 Oct 2019 13:23:49 +0200
Subject: [PATCH] [FIX] context: solve tricky concurrency issue

part of #330
---
 src/context.ts        | 29 ++++++++++++++++++++++-------
 tests/context.test.ts |  6 +++---
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/src/context.ts b/src/context.ts
index 3748735b..6e381c62 100644
--- a/src/context.ts
+++ b/src/context.ts
@@ -15,7 +15,7 @@ import { onWillUnmount } from "./hooks";
 export class Context extends EventBus {
   state: any;
   observer: Observer;
-  id: number = 1;
+  rev: number = 1;
   // mapping from component id to last observed context id
   mapping: { [componentId: number]: number } = {};
 
@@ -46,13 +46,13 @@ export class Context extends EventBus {
    * with the same depth in parallel.
    */
   async __notifyComponents() {
-    const id = ++this.id;
+    const rev = ++this.rev;
     const subs = this.subscriptions.update || [];
     for (let i = 0, iLen = subs.length; i < iLen; i++) {
       const sub = subs[i];
       const shouldCallback = sub.owner ? sub.owner.__owl__.isMounted : true;
       if (shouldCallback) {
-        const render = sub.callback.call(sub.owner, id);
+        const render = sub.callback.call(sub.owner, rev);
         scheduler.flush();
         await render;
       }
@@ -76,15 +76,30 @@ export function useContextWithCB(ctx: Context, component: Component<any, any>, m
   if (id in mapping) {
     return ctx.state;
   }
+  if (!__owl__.observer) {
+    __owl__.observer = new Observer();
+    __owl__.observer.notifyCB = component.render.bind(component);
+  }
+  const currentCB = __owl__.observer.notifyCB;
+  __owl__.observer.notifyCB = function () {
+      if (ctx.rev > mapping[id]) {
+          // in this case, the context has been updated since we were rendering
+          // last, and we do not need to render here with the observer. A
+          // rendering is coming anyway, with the correct props.
+          return;
+      }
+      currentCB();
+  }
+
   mapping[id] = 0;
   const renderFn = __owl__.renderFn;
   __owl__.renderFn = function(comp, params) {
-    mapping[id] = ctx.id;
+    mapping[id] = ctx.rev;
     return renderFn(comp, params);
   };
-  ctx.on("update", component, async contextId => {
-    if (mapping[id] < contextId) {
-      mapping[id] = contextId;
+  ctx.on("update", component, async contextRev => {
+    if (mapping[id] < contextRev) {
+      mapping[id] = contextRev;
       await method();
     }
   });
diff --git a/tests/context.test.ts b/tests/context.test.ts
index 311ad75c..124b9cbd 100644
--- a/tests/context.test.ts
+++ b/tests/context.test.ts
@@ -174,7 +174,7 @@ describe("Context", () => {
     expect(testContext.subscriptions.update.length).toBe(0);
   });
 
-  test.skip("concurrent renderings", async () => {
+  test("concurrent renderings", async () => {
     const testContext = new Context({ x: { n: 1 }, key: "x" });
     const def = makeDeferred();
     let stateC;
@@ -207,7 +207,7 @@ describe("Context", () => {
 
     expect(fixture.innerHTML).toBe("<div><p><span>1a</span></p></div>");
     testContext.state.key = "y";
-    testContext.state.y = 2;
+    testContext.state.y = {n: 2};
     delete testContext.state.x;
     await nextTick();
 
@@ -219,6 +219,6 @@ describe("Context", () => {
     def.resolve();
     await nextTick();
 
-    expect(fixture.innerHTML).toBe("<div><p><span>1a</span></p></div>");
+    expect(fixture.innerHTML).toBe("<div><p><span>2b</span></p></div>");
   });
 });