fix(zone.js): remove abort listener once fetch is settled (angular#57882)

This commit updates the `fetch` patch for zone.js. Currently, we're attaching an
`abort` event listener on the signal (when it's provided) and never removing it.
We should be good citizens and remove event listeners whenever objects need to be
properly collected. In Firefox, when saving a heap snapshot and running it through
`fxsnapshot`, querying `AbortSignal` will print a so-called "CaptureMap" with a list
of "lambdas," indicating that the signal is not garbage collected because of the event
listener lambda function.

PR Close angular#57882

Author: arturovt

packages/zone.js/lib/common/fetch.ts

@@ -99,22 +99,31 @@ export function patchFetch(Zone: ZoneType): void {
       options.signal = fetchSignal;
       args[1] = options;
 
+      let onAbort: () => void;
       if (signal) {
         const nativeAddEventListener =
           signal[Zone.__symbol__('addEventListener') as 'addEventListener'] ||
           signal.addEventListener;
 
-        nativeAddEventListener.call(
-          signal,
-          'abort',
-          function () {
-            ac!.abort();
-          },
-          {once: true},
-        );
+        onAbort = () => ac!.abort();
+        nativeAddEventListener.call(signal, 'abort', onAbort, {once: true});
       }
 
-      return createFetchTask('fetch', {fetchArgs: args} as FetchTaskData, fetch, this, args, ac);
+      return createFetchTask(
+        'fetch',
+        {fetchArgs: args} as FetchTaskData,
+        fetch,
+        this,
+        args,
+        ac,
+      ).finally(() => {
+        // We need to be good citizens and remove the `abort` listener once
+        // the fetch is settled. The `abort` listener may not be called at all,
+        // which means the event listener closure would retain a reference to
+        // the `ac` object even if it goes out of scope. Since browser's garbage
+        // collectors work differently, some may not be smart enough to collect a signal.
+        signal?.removeEventListener('abort', onAbort);
+      });
     };
 
     if (OriginalResponse?.prototype) {

packages/zone.js/test/common/fetch.spec.ts

@@ -169,6 +169,10 @@ describe(
                       'invokeTask:fetch:macroTask',
                       'scheduleTask:Promise.then:microTask',
                       'invokeTask:Promise.then:microTask',
+
+                      // This is the `finally` task, which is used for cleanup.
+                      'scheduleTask:Promise.then:microTask',
+                      'invokeTask:Promise.then:microTask',
                     ]);
                     done();
                   },
@@ -194,6 +198,11 @@ describe(
                       'invokeTask:fetch:macroTask',
                       'scheduleTask:Promise.then:microTask',
                       'invokeTask:Promise.then:microTask',
+
+                      // This is the `finally` task, which is used for cleanup.
+                      'scheduleTask:Promise.then:microTask',
+                      'invokeTask:Promise.then:microTask',
+
                       // Please refer to the issue link above. Previously, `Response` methods were not
                       // patched by zone.js, and their return values were considered only as
                       // microtasks (not macrotasks). The Angular zone stabilized prematurely,