From 2cca0bd81938b0555e50cb5a1d14a8bb49e42628 Mon Sep 17 00:00:00 2001 From: "Julien Carion (juca)" Date: Wed, 14 Jun 2023 15:23:31 +0200 Subject: [PATCH] [FIX] devtools: Fix race conditions in tree lodaing This commit fixes some race conditions that were happening due to the loading of the tree being sometimes triggered at the same moment as the DOM patch. This would result in some destroyed component still being present in the devtools' tree. This commit also ensures that the loading of the tree is synchronous in regard to the component details loading. --- .../devtools/src/devtools_app/store/store.js | 23 +++++++-------- .../page_scripts/owl_devtools_global_hook.js | 28 ++++++++----------- 2 files changed, 21 insertions(+), 30 deletions(-) diff --git a/tools/devtools/src/devtools_app/store/store.js b/tools/devtools/src/devtools_app/store/store.js index 26898b66..1e42d77f 100644 --- a/tools/devtools/src/devtools_app/store/store.js +++ b/tools/devtools/src/devtools_app/store/store.js @@ -102,20 +102,17 @@ export const store = reactive({ if (IS_FIREFOX) { await evalInWindow("window.$0 = $0;", this.activeFrame); } - const apps = await evalFunctionInWindow( + const [apps, component] = await evalFunctionInWindow( "getComponentsTree", - fromOld && this.activeComponent ? [this.activeComponent.path, this.apps] : [], + fromOld && this.activeComponent + ? [this.activeComponent.path, this.apps, this.activeComponent] + : [], this.activeFrame ); this.apps = apps ? apps : []; if (!fromOld && this.settings.expandByDefault) { this.apps.forEach((tree) => expandNodes(tree, true)); } - const component = await evalFunctionInWindow( - "getComponentDetails", - fromOld && this.activeComponent ? [this.activeComponent.path, this.activeComponent] : [], - this.activeFrame - ); this.activeComponent = component; }, @@ -671,7 +668,7 @@ async function init() { }, 500); } -let flushRendersTimeout = false; +let rootRendersTimeout = false; // Connect to the port to communicate to the background script browserInstance.runtime.onConnect.addListener((port) => { if (port.name === "OwlDevtoolsPort_" + store.devtoolsId) { @@ -694,9 +691,9 @@ browserInstance.runtime.onConnect.addListener((port) => { if (msg.type === "RefreshApps") { store.loadComponentsTree(true); } - // When message of type Flush is received, overwrite the component tree with the new one from page - // A flush message is sent everytime a component is rendered on the page - if (msg.type === "Flush") { + // When message of type Complete is received, overwrite the component tree with the new one from page + // A Complete message is sent everytime a root render is triggered on the page + if (msg.type === "Complete") { if (msg.origin.frame !== store.activeFrame) { return; } @@ -705,8 +702,8 @@ browserInstance.runtime.onConnect.addListener((port) => { } // This determines which components will have a short highlight effect in the tree to indicate they have been rendered store.renderPaths.add(JSON.stringify(msg.data)); - clearTimeout(flushRendersTimeout); - flushRendersTimeout = setTimeout(() => { + clearTimeout(rootRendersTimeout); + rootRendersTimeout = setTimeout(() => { store.renderPaths.clear(); }, 100); store.loadComponentsTree(true); diff --git a/tools/devtools/src/page_scripts/owl_devtools_global_hook.js b/tools/devtools/src/page_scripts/owl_devtools_global_hook.js index ff6a0a3e..d7b48300 100644 --- a/tools/devtools/src/page_scripts/owl_devtools_global_hook.js +++ b/tools/devtools/src/page_scripts/owl_devtools_global_hook.js @@ -15,8 +15,6 @@ // in __OWL_DEVTOOLS__ this.toRaw = window.__OWL_DEVTOOLS__.toRaw ?? window.owl?.toRaw; this.reactive = window.__OWL_DEVTOOLS__.reactive ?? window.owl?.reactive; - // Set to keep track of the fibers that are in the flush queue - this.queuedFibers = new WeakSet(); // Set to keep track of the HTML elements we added to the page this.addedElements = []; // To keep track of the succession order of the render events @@ -267,19 +265,6 @@ app.scheduler.constructor.prototype.flush = function () { // Used to know when a render is triggered inside the flush method or not inFlush = true; - [...this.tasks].map((fiber) => { - if (fiber.counter === 0 && !self.queuedFibers.has(fiber)) { - self.queuedFibers.add(fiber); - const path = self.getComponentPath(fiber.node); - //Add a functionnality to the flush function which sends a message to the window every time it is triggered. - window.top.postMessage({ - source: "owl-devtools", - type: "Flush", - data: path, - origin: { frame: self.frame }, - }); - } - }); originalFlush.call(this, ...arguments); inFlush = false; }; @@ -380,6 +365,14 @@ const original_Complete = self.RootFiber.prototype.complete; self.RootFiber.prototype.complete = function () { original_Complete.call(this, ...arguments); + const path = self.getComponentPath(this.node); + //Add a functionnality to the complete function which sends a message to the window every time it is triggered. + window.top.postMessage({ + source: "owl-devtools", + type: "Complete", + data: path, + origin: { frame: self.frame }, + }); if (self.recordEvents) { window.top.postMessage({ source: "owl-devtools", @@ -1556,7 +1549,7 @@ } // Returns the tree of components of the inspected page in a parsed format // Use inspectedPath to specify the path of the selected component - getComponentsTree(inspectedPath = null, oldTrees = null) { + getComponentsTree(inspectedPath = null, oldTrees = null, oldDetails = null) { const appsArray = [...this.apps]; const trees = appsArray.map((app, index) => { let oldTree; @@ -1609,7 +1602,8 @@ } return appNode; }); - return trees ? trees : []; + const component = this.getComponentDetails(inspectedPath, oldDetails); + return trees ? [trees, component] : []; } // Recursively fills the components tree as a parsed version fillTree(appNode, treeNode, inspectedPathString, oldBranch) {