[FIX] web: wait a microtick before starting services

That way, we ensure that all synchronously defined services are
added to the registry, i.e. if a service depends on another one,
its dependency is present, regardless the order they are added to
the registry.

Closes odoo-dev/odoo#967

Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>

addons/web/static/src/env.js

@@ -60,7 +60,7 @@ export async function startServices(env) {
     serviceRegistry.on("UPDATE", null, async (payload) => {
         // Wait for all synchronous code so that if new services that depend on
         // one another are added to the registry, they're all present before we
-        // start them.
+        // start them regardless of the order they're added to the registry.
         await Promise.resolve();
         const { operation, key: name, value: service } = payload;
         if (operation === "delete") {
@@ -76,10 +76,14 @@ export async function startServices(env) {
             await _startServices(env, toStart);
         }
     });
+    // Wait for all synchronous code so that if new services that depend on
+    // one another are added to the registry, they're all present before we
+    // start them regardless of the order they're added to the registry.
+    await Promise.resolve();
     await _startServices(env, toStart);
 }
 
-async function _startServices(env, toStart, timeoutId) {
+async function _startServices(env, toStart) {
     const services = env.services;
     for (const [name, service] of serviceRegistry.getEntries()) {
         if (!(name in services)) {

addons/web/static/tests/env_tests.js

@@ -1,12 +1,17 @@
 /** @odoo-module **/
 
+import { makeEnv, startServices } from "@web/env";
 import { registry } from "@web/core/registry";
-import { makeTestEnv } from "./helpers/mock_env";
+import {
+    clearRegistryWithCleanup,
+    clearServicesMetadataWithCleanup,
+    makeTestEnv,
+} from "./helpers/mock_env";
 import { makeDeferred, nextTick, patchWithCleanup } from "./helpers/utils";
 
 const serviceRegistry = registry.category("services");
 
-QUnit.module("deployServices");
+QUnit.module("env");
 
 QUnit.test("can start a service", async (assert) => {
     serviceRegistry.add("test", {
@@ -69,6 +74,7 @@ QUnit.test("can start an asynchronous service", async (assert) => {
         },
     });
     const prom = makeTestEnv();
+    await Promise.resolve(); // Wait for startServices
     assert.verifySteps(["before"]);
     def.resolve(15);
     const env = await prom;
@@ -173,3 +179,66 @@ QUnit.test("get an object containing dependencies as second arg", async (assert)
     });
     await makeTestEnv();
 });
+
+QUnit.test(
+    "startServices: throws if all dependencies are not met in the same microtick as the call",
+    async function (assert) {
+        assert.expect(3);
+        clearRegistryWithCleanup(serviceRegistry);
+        clearServicesMetadataWithCleanup();
+
+        const serviceA = {
+            start() {
+                return "a";
+            },
+        };
+        const serviceB = {
+            dependencies: ["a"],
+            start() {
+                return "b";
+            },
+        };
+        const env = makeEnv();
+
+        serviceRegistry.add("b", serviceB);
+        const prom = startServices(env);
+        await Promise.resolve();
+        assert.rejects(prom, "Some services could not be started: b. Missing dependencies: a");
+        assert.deepEqual(env.services, {});
+
+        serviceRegistry.add("a", serviceA);
+        await startServices(env);
+        assert.deepEqual(env.services, { a: "a", b: "b" });
+    }
+);
+
+QUnit.test(
+    "startServices: waits for all synchronous code before attempting to start services",
+    async function (assert) {
+        assert.expect(1);
+        clearRegistryWithCleanup(serviceRegistry);
+        clearServicesMetadataWithCleanup();
+
+        const serviceA = {
+            start() {
+                return "a";
+            },
+        };
+        const serviceB = {
+            dependencies: ["a"],
+            start() {
+                return "b";
+            },
+        };
+
+        const env = makeEnv();
+        serviceRegistry.add("b", serviceB);
+        const prom = startServices(env);
+        // Dependency added in the same microtick doesn't cause startServices to throw even if it was added after the call
+        // (eg, a module is defined after main.js)
+        serviceRegistry.add("a", serviceA);
+
+        await prom;
+        assert.deepEqual(env.services, { a: "a", b: "b" });
+    }
+);