Bug 1710674 - [devtools] Fix target-mixin resource cache mechanism. r=ochameau,bomsy.

The event listeners that were set in the constructor were not
actually removed in the `on` method, as we weren't using the
proper function reference.
Furthermore, the event listeners were removed only if some
resources were put in the cache, which could lead to some
buggy behavior when watching/unwatching multiple times.

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

devtools/client/fronts/targets/target-mixin.js

@@ -59,24 +59,37 @@ function TargetMixin(parentClass) {
 
       // In order to avoid destroying the `_resourceCache[event]`, we need to call `super.on()`
       // instead of `this.on()`.
-      super.on(
+      const offResourceAvailable = super.on(
         "resource-available-form",
         this._onResourceEvent.bind(this, "resource-available-form")
       );
-      super.on(
+      const offResourceUpdated = super.on(
         "resource-updated-form",
         this._onResourceEvent.bind(this, "resource-updated-form")
       );
+
+      this._offResourceEvent = new Map([
+        ["resource-available-form", offResourceAvailable],
+        ["resource-updated-form", offResourceUpdated],
+      ]);
     }
 
     on(eventName, listener) {
-      const cachedEvents = ["resource-available-form", "resource-updated-form"];
-      if (cachedEvents.includes(eventName) && this._resourceCache[eventName]) {
-        this.off(eventName, this._onResourceEvent.bind(this, eventName));
-        for (const cache of this._resourceCache[eventName]) {
-          listener(cache);
+      if (this._offResourceEvent.has(eventName)) {
+        // If a callsite sets an event listener for resource-(available|update)-form:
+
+        // we want to remove the listener we set here in the constructor…
+        const off = this._offResourceEvent.get(eventName);
+        this._offResourceEvent.delete(eventName);
+        off();
+
+        // …and call the new listener with the resources that were put in the cache.
+        if (this._resourceCache[eventName]) {
+          for (const cache of this._resourceCache[eventName]) {
+            listener(cache);
+          }
+          delete this._resourceCache[eventName];
         }
-        delete this._resourceCache[eventName];
       }
 
       return super.on(eventName, listener);
@@ -632,6 +645,7 @@ function TargetMixin(parentClass) {
       }
 
       this.threadFront = null;
+      this._offResourceEvent = null;
 
       // This event should be emitted before calling super.destroy(), because
       // super.destroy() will remove all event listeners attached to this front.

devtools/shared/commands/resource/tests/browser_resources_watch_unwatch_multiple.js

@@ -59,8 +59,7 @@ add_task(async function() {
   });
   is(
     resources.length,
-    // We should get only 1 resource. This will be fixed in Bug 1710674
-    2,
+    1,
     "we retrieve the expected number of existing resources"
   );
   is(