Bug 1685268 - [devtools] Don't clear StyleEditor UI on redundant dom-loading event. r=jdescottes,ochameau.

With the previous patches in this queue, it looks like the
timing is stlightly modified, and on initial load we can
reveive stylesheet resources _before_ the intial dom-loading
event.
Since we're using dom-loading to clear the UI, we might destroy
stylesheets that we shouldn't.
We fix that in 2 folds:
- clear the UI in onTargetAvailable, which we do receive before any resources
- add a flag to the `dom-loading` resource when we're already going to be triggering
  onTargetAvailable for a given interaction (toolbox opening, cross-process
  navigation or any navigation when target will follow the global lifecycle).

Differential Revision: https://phabricator.services.mozilla.com/D109471

devtools/client/styleeditor/StyleEditorUI.jsm

@@ -1185,6 +1185,7 @@ StyleEditorUI.prototype = {
   },
 
   async _onResourceAvailable(resources) {
+    const promises = [];
     for (const resource of resources) {
       if (
         resource.resourceType === this._toolbox.resourceWatcher.TYPES.STYLESHEET
@@ -1195,8 +1196,7 @@ StyleEditorUI.prototype = {
           // In case of reloading/navigating and panel's opening
           this._loadingStyleSheets.push(onStyleSheetHandled);
         }
-
-        await onStyleSheetHandled;
+        promises.push(onStyleSheetHandled);
         continue;
       }
 
@@ -1208,15 +1208,26 @@ StyleEditorUI.prototype = {
         // will-navigate doesn't work when we navigate to a new process,
         // and for now, onTargetAvailable/onTargetDestroyed doesn't fire on navigation and
         // only when navigating to another process.
-        // So we fallback on DOCUMENT_EVENTS to be notified when we navigates. When we
-        // navigate within the same process as well as when we navigate to a new process.
+        // So we fallback on DOCUMENT_EVENTS to be notified when we navigates.
+        // The only catch is that when starting up a DevToolsServer, a dom-loading event
+        // is fired, and it might come _after_ the stylesheet resources; and in such case
+        // we shouldn't clear the stylesheets that where already handled.
+        // So here we bail out if we get this kind of event. At the moment, this will only
+        // happen for the initial target and for target switching (and luckily, in those
+        // case onTargetAvailable is called before we get any stylesheets).
+        // This means we still want to clear the UI on dom-loading for same-process navigation.
         // (We would probably revisit that in bug 1632141)
+        if (resource.shouldBeIgnoredAsRedundantWithTargetAvailable) {
+          continue;
+        }
+
         this._startLoadingStyleSheets();
         this._clear();
       } else if (resource.name === "dom-complete") {
-        await this._waitForLoadingStyleSheets();
+        promises.push(this._waitForLoadingStyleSheets());
       }
     }
+    await Promise.all(promises);
   },
 
   async _onResourceUpdated(updates) {
@@ -1254,6 +1265,8 @@ StyleEditorUI.prototype = {
 
   async _onTargetAvailable({ targetFront }) {
     if (targetFront.isTopLevel) {
+      this._startLoadingStyleSheets();
+      this._clear();
       await this.initializeHighlighter(targetFront);
     }
   },

devtools/server/actors/resources/document-event.js

@@ -27,12 +27,18 @@ class DocumentEventWatcher {
       return;
     }
 
-    const onDocumentEvent = (name, time) => {
+    const onDocumentEvent = (
+      name,
+      time,
+      // This is only passed for dom-loading event
+      shouldBeIgnoredAsRedundantWithTargetAvailable
+    ) => {
       onAvailable([
         {
           resourceType: DOCUMENT_EVENT,
           name,
           time,
+          shouldBeIgnoredAsRedundantWithTargetAvailable,
         },
       ]);
     };

devtools/server/actors/webconsole/listeners/document-events.js

@@ -27,17 +27,39 @@ exports.DocumentEventsListener = DocumentEventsListener;
 DocumentEventsListener.prototype = {
   listen() {
     EventEmitter.on(this.targetActor, "window-ready", this.onWindowReady);
-    this.onWindowReady({ window: this.targetActor.window, isTopLevel: true });
+    this.onWindowReady({
+      window: this.targetActor.window,
+      isTopLevel: true,
+      // Flag the very first dom-loading event, which is about the top target and may come
+      // after some other already existing resources.
+      shouldBeIgnoredAsRedundantWithTargetAvailable: true,
+    });
   },
 
-  onWindowReady({ window, isTopLevel }) {
+  onWindowReady({
+    window,
+    isTopLevel,
+    shouldBeIgnoredAsRedundantWithTargetAvailable,
+  }) {
     // Ignore iframes
     if (!isTopLevel) {
       return;
     }
 
     const time = window.performance.timing.navigationStart;
-    this.emit("dom-loading", time);
+    this.emit(
+      "dom-loading",
+      time,
+      // As dom-loading is often used to clear the panel on navigation, and is typically
+      // sent before any other resource, we need to add a hint so the client knows when
+      // then event can be ignored.
+      // We should also ignore them if the Target was created via a JSWindowActor and is
+      // destroyed when the WindowGlobal is destroyed (i.e. when we navigate or reload),
+      // as this will come late and is redundant with onTargetAvailable.
+      shouldBeIgnoredAsRedundantWithTargetAvailable ||
+        (this.targetActor.isTopLevel &&
+          this.targetActor.followWindowGlobalLifecycle)
+    );
 
     const { readyState } = window.document;
     if (readyState != "interactive" && readyState != "complete") {

devtools/shared/resources/tests/browser_resources_document_events.js

@@ -39,6 +39,12 @@ async function testDocumentEventResources() {
     true,
     "Document events are fired even when the document was already loaded"
   );
+  let domLoadingResource = await onLoadingAtInit;
+  is(
+    domLoadingResource.shouldBeIgnoredAsRedundantWithTargetAvailable,
+    true,
+    "shouldBeIgnoredAsRedundantWithTargetAvailable is true for already loaded page"
+  );
 
   info("Check whether the document events are fired correctly when reloading");
   const onLoadingAtReloaded = listener.once("dom-loading");
@@ -52,6 +58,13 @@ async function testDocumentEventResources() {
   );
   ok(true, "Document events are fired after reloading");
 
+  domLoadingResource = await onLoadingAtReloaded;
+  is(
+    domLoadingResource.shouldBeIgnoredAsRedundantWithTargetAvailable,
+    undefined,
+    "shouldBeIgnoredAsRedundantWithTargetAvailable is not set after reloading"
+  );
+
   targetList.destroy();
   await client.close();
 }