[FIX] component: async issue

This is a tricky commit. The key point is that the Fiber.complete
method, which commits a rendering to the DOM works like this: it
traverses the component tree, patch the corresponding DOM for each
component, calls the mounted/destroy hooks, and reset the currentfiber
of components to null, all synchronously.

However, this means that it is possible for components to initiate a
rendering (which create a new currentFiber) before the currentFiber is
reset to null, so the internal state of owl is corrupted. This can
occurs in a crash, as in the test that accompanies this commit.

To fix this, we take care of resetting the currentFiber first, while we
walk the component tree. Then, the internal state is always consistent
(i.e. a currentFiber to null means that there is no pending rendering)

closes #904
This commit is contained in:
Géry Debongnie
2021-10-04 11:50:20 +02:00
committed by aab-odoo
parent 37313c47a3
commit 64db7777dd
3 changed files with 58 additions and 5 deletions
+1 -1
View File
@@ -400,6 +400,7 @@ export class Component<Props extends {} = any, T extends Env = Env> {
if (currentFiber && !currentFiber.isRendered && !currentFiber.isCompleted) {
return scheduler.addFiber(currentFiber.root);
}
// if we aren't mounted at this point, it implies that there is a
// currentFiber that is already rendered (isRendered is true), so we are
// about to be mounted
@@ -505,7 +506,6 @@ export class Component<Props extends {} = any, T extends Env = Env> {
const __owl__ = this.__owl__;
__owl__.status = STATUS.MOUNTED;
__owl__.currentFiber = null;
this.mounted();
if (__owl__.mountedCB) {
__owl__.mountedCB();
+1 -4
View File
@@ -198,6 +198,7 @@ export class Fiber {
// build patchQueue
const patchQueue: Fiber[] = [];
const doWork: (Fiber) => Fiber | null = function (f) {
f.component.__owl__.currentFiber = null;
patchQueue.push(f);
return f.child;
};
@@ -255,10 +256,6 @@ export class Fiber {
component.__owl__.pvnode!.elm = component.__owl__.vnode!.elm;
}
}
const compOwl = component.__owl__;
if (fiber === compOwl.currentFiber) {
compOwl.currentFiber = null;
}
}
// insert into the DOM (mount case)
+56
View File
@@ -1388,6 +1388,62 @@ describe("async rendering", () => {
expect(fixture.innerHTML).toBe("<span>4</span>");
});
test("calling render in destroy", async () => {
let a: any = null;
let c: any = null;
class C extends Component {
static template = xml`
<div>
<t t-esc="props.fromA"/>
</div>`;
}
let flag = false;
class B extends Component {
static template = xml`<C fromA="props.fromA"/>`;
static components = { C };
setup() {
c = this;
}
mounted() {
if (flag) {
this.render();
} else {
flag = true;
}
}
willUnmount() {
c.render();
}
}
class A extends Component {
static template = xml`<B t-key="key" fromA="state"/>`;
static components = { B };
state = "a";
key = 1;
setup() {
a = this;
}
}
const parent = new A();
await parent.mount(fixture);
expect(fixture.innerHTML).toBe("<div>a</div>");
a.state = "A";
a.key = 2;
await a.render();
// this nextTick is critical, otherwise jest may silently swallow errors
await nextTick();
expect(fixture.innerHTML).toBe("<div>A</div>");
});
test("change state and call manually render: no unnecessary rendering", async () => {
class Widget extends Component {
static template = xml`<div><t t-esc="state.val"/></div>`;