[FIX] web: fix useService binding service methods when it should not

Previously, the useService hook would call _protectMethod, which wraps
async methods so that any call to them after the component has been
destroyed throws an error, and any call that was started while the
component was alive but returned after the component was destroyed would
return a pending promise such that no more component code is executed.

In doing so, it would for some reason bind all the methods to the
service, this prevents some functionalities where a method would return
a differently configured version of that same service, on which you can
call the methods again (eg: orm.silent), as the modified service's
methods would not be bound to that modified service.

This commit fixes this issue by not binding the methods eagerly, and
introduces a test for all these behaviours.

closes odoo/odoo#96121

X-original-commit: 518a93a5487f62bd9b460412d4775cd0ac80a262
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
Signed-off-by: Samuel Degueldre <sad@odoo.com>

addons/web/static/src/core/utils/hooks.js

@@ -161,12 +161,12 @@ export function useListener(eventName, querySelector, handler, options = {}) {
 // useService
 // -----------------------------------------------------------------------------
 
-function _protectMethod(component, caller, fn) {
-    return async (...args) => {
+function _protectMethod(component, fn) {
+    return async function (...args) {
         if (status(component) === "destroyed") {
             throw new Error("Component is destroyed");
         }
-        const result = await fn.call(caller, ...args);
+        const result = await fn.call(this, ...args);
         return status(component) === "destroyed" ? new Promise(() => {}) : result;
     };
 }
@@ -186,12 +186,12 @@ export function useService(serviceName) {
     const service = services[serviceName];
     if (serviceName in SERVICES_METADATA) {
         if (service instanceof Function) {
-            return _protectMethod(component, null, service);
+            return _protectMethod(component, service);
         } else {
             const methods = SERVICES_METADATA[serviceName];
             const result = Object.create(service);
             for (const method of methods) {
-                result[method] = _protectMethod(component, service, service[method]);
+                result[method] = _protectMethod(component, service[method]);
             }
             return result;
         }

addons/web/static/tests/core/utils/hooks_tests.js

@@ -4,7 +4,14 @@ import { uiService } from "@web/core/ui/ui_service";
 import { useAutofocus, useBus, useListener, useService } from "@web/core/utils/hooks";
 import { registry } from "@web/core/registry";
 import { makeTestEnv } from "@web/../tests/helpers/mock_env";
-import { click, destroy, getFixture, mount, nextTick } from "@web/../tests/helpers/utils";
+import {
+    click,
+    destroy,
+    getFixture,
+    makeDeferred,
+    mount,
+    nextTick,
+} from "@web/../tests/helpers/utils";
 import { LegacyComponent } from "@web/legacy/legacy_component";
 
 const { Component, onMounted, xml } = owl;
@@ -255,5 +262,80 @@ QUnit.module("utils", () => {
             const comp = await mount(MyComponent, target, { env });
             assert.strictEqual(comp.toyService, null);
         });
+
+        QUnit.test("useService: async service with protected methods", async function (assert) {
+            let nbCalls = 0;
+            let def = makeDeferred();
+            class MyComponent extends Component {
+                setup() {
+                    this.objectService = useService("object_service");
+                    this.functionService = useService("function_service");
+                }
+            }
+            MyComponent.template = xml`<div/>`;
+
+            serviceRegistry.add("object_service", {
+                name: "object_service",
+                async: ["asyncMethod"],
+                start() {
+                    return {
+                        async asyncMethod() {
+                            nbCalls++;
+                            await def;
+                            return this;
+                        },
+                    };
+                },
+            });
+
+            serviceRegistry.add("function_service", {
+                name: "function_service",
+                async: true,
+                start() {
+                    return async function asyncFunc() {
+                        nbCalls++;
+                        await def;
+                        return this;
+                    };
+                },
+            });
+
+            const env = await makeTestEnv();
+            const target = getFixture();
+
+            const comp = await mount(MyComponent, target, { env });
+            // Functions and methods have the correct this
+            def.resolve();
+            assert.deepEqual(await comp.objectService.asyncMethod(), comp.objectService);
+            assert.deepEqual(await comp.objectService.asyncMethod.call("boundThis"), "boundThis");
+            assert.deepEqual(await comp.functionService(), comp);
+            assert.deepEqual(await comp.functionService.call("boundThis"), "boundThis");
+            assert.strictEqual(nbCalls, 4);
+            // Functions that were called before the component is destroyed but resolved after never resolve
+            let nbResolvedProms = 0;
+            def = makeDeferred();
+            comp.objectService.asyncMethod().then(() => nbResolvedProms++);
+            comp.objectService.asyncMethod.call("boundThis").then(() => nbResolvedProms++);
+            comp.functionService().then(() => nbResolvedProms++);
+            comp.functionService.call("boundThis").then(() => nbResolvedProms++);
+            assert.strictEqual(nbCalls, 8);
+            comp.__owl__.app.destroy();
+            def.resolve();
+            await nextTick();
+            assert.strictEqual(
+                nbResolvedProms,
+                0,
+                "The promises returned by the calls should never resolve"
+            );
+            // Calling the functions after the destruction rejects the promise
+            assert.rejects(comp.objectService.asyncMethod(), "Component is destroyed");
+            assert.rejects(
+                comp.objectService.asyncMethod.call("boundThis"),
+                "Component is destroyed"
+            );
+            assert.rejects(comp.functionService(), "Component is destroyed");
+            assert.rejects(comp.functionService.call("boundThis"), "Component is destroyed");
+            assert.strictEqual(nbCalls, 8);
+        });
     });
 });