fix: address GhostMaps resource and source ownership review findings

src/layers/ghostMaps/index.js

@@ -195,7 +195,9 @@ export function createGhostMapsLayer({
   /** Build one product's data source off-scene, then swap it in. */
   async function load(product, signal) {
     const bytes = await source.getFile(product.id, product.version, { signal });
+    signal.throwIfAborted();
     const entries = await readZipEntries(bytes);
+    signal.throwIfAborted();
     const main = kmzMainDocument(entries);
     if (!main) throw new Error(msg('That KMZ holds no KML document.'));
     const urls = [];
@@ -210,117 +212,118 @@ export function createGhostMapsLayer({
         ? resolvePacked(href)
         : null;
     };
-    let parsed;
     try {
-      parsed = readGhostMapsKml(
+      const parsed = readGhostMapsKml(
         parseXml(decodeXml(entries.get(main)), 'KML'),
         resolveImage,
       );
-    } catch (error) {
-      urls.forEach((url) => URL.revokeObjectURL(url));
-      throw error;
-    }
-    entries.clear();
-    signal.throwIfAborted();
-    const color = PRODUCT_COLORS[product.id] || ACCENT;
-    const dataSource = new Cesium.CustomDataSource(
-      `${GHOSTMAPS_LAYER_ID}:${product.id}`,
-    );
-    const features = [];
-    const pins = [];
-    dataSource.entities.suspendEvents();
-    parsed.features.forEach((feature, index) => {
-      const key = `${product.id}:${index}`;
-      const { style } = feature;
-      feature.geometries.forEach((geometry, part) => {
-        const id = `${PICK_PREFIX}${key}:${part}`;
-        if (geometry.type === 'point') {
-          const position = Cesium.Cartesian3.fromDegrees(...geometry.position);
-          const entity = dataSource.entities.add({
-            id,
-            position,
-            billboard: {
-              image: style.icon || markerImage(color),
-              scale: Math.min(
-                1.5,
-                Math.max(0.35, (style.iconScale ?? 1) * 0.7),
-              ),
-              color:
-                style.icon && style.iconColor
-                  ? cesiumColor(style.iconColor)
-                  : undefined,
-              heightReference: Cesium.HeightReference.CLAMP_TO_GROUND,
-              verticalOrigin: Cesium.VerticalOrigin.CENTER,
-              // Ground-clamped icons would sink into terrain and 3D tiles; the
-              // horizon cull hides the far side instead (see `_horizon`).
-              disableDepthTestDistance: Number.POSITIVE_INFINITY,
-            },
-          });
-          pins.push({
-            position,
-            get show() {
-              return entity.show;
-            },
-            set show(value) {
-              entity.show = value;
-            },
-          });
-        } else if (geometry.type === 'line') {
-          dataSource.entities.add({
-            id,
-            polyline: {
-              positions: Cesium.Cartesian3.fromDegreesArray(
-                geometry.positions.flat(),
-              ),
-              width: Math.min(8, Math.max(1.5, style.lineWidth ?? 2)),
-              material: cesiumColor(style.lineColor, color),
-              clampToGround: true,
-            },
-          });
-        } else {
-          const ring = (points) =>
-            Cesium.Cartesian3.fromDegreesArray(points.flat());
-          dataSource.entities.add({
-            id,
-            polygon: {
-              hierarchy: new Cesium.PolygonHierarchy(
-                ring(geometry.outer),
-                geometry.holes.map(
-                  (hole) => new Cesium.PolygonHierarchy(ring(hole)),
+      entries.clear();
+      signal.throwIfAborted();
+      const color = PRODUCT_COLORS[product.id] || ACCENT;
+      const dataSource = new Cesium.CustomDataSource(
+        `${GHOSTMAPS_LAYER_ID}:${product.id}`,
+      );
+      const features = [];
+      const pins = [];
+      dataSource.entities.suspendEvents();
+      parsed.features.forEach((feature, index) => {
+        const key = `${product.id}:${index}`;
+        const { style } = feature;
+        feature.geometries.forEach((geometry, part) => {
+          const id = `${PICK_PREFIX}${key}:${part}`;
+          if (geometry.type === 'point') {
+            const position = Cesium.Cartesian3.fromDegrees(
+              ...geometry.position,
+            );
+            const entity = dataSource.entities.add({
+              id,
+              position,
+              billboard: {
+                image: style.icon || markerImage(color),
+                scale: Math.min(
+                  1.5,
+                  Math.max(0.35, (style.iconScale ?? 1) * 0.7),
                 ),
-              ),
-              material:
-                style.fill === false
-                  ? Cesium.Color.TRANSPARENT
-                  : cesiumColor(
-                      style.polyColor,
-                      color,
-                      style.polyColor ? undefined : 0.25,
-                    ),
-              classificationType: Cesium.ClassificationType.BOTH,
-            },
-          });
-          if (style.outline !== false)
+                color:
+                  style.icon && style.iconColor
+                    ? cesiumColor(style.iconColor)
+                    : undefined,
+                heightReference: Cesium.HeightReference.CLAMP_TO_GROUND,
+                verticalOrigin: Cesium.VerticalOrigin.CENTER,
+                // Ground-clamped icons would sink into terrain and 3D tiles; the
+                // horizon cull hides the far side instead (see `_horizon`).
+                disableDepthTestDistance: Number.POSITIVE_INFINITY,
+              },
+            });
+            pins.push({
+              position,
+              get show() {
+                return entity.show;
+              },
+              set show(value) {
+                entity.show = value;
+              },
+            });
+          } else if (geometry.type === 'line') {
             dataSource.entities.add({
-              id: `${id}:outline`,
+              id,
               polyline: {
-                positions: ring([...geometry.outer, geometry.outer[0]]),
-                width: Math.min(6, Math.max(1.5, style.lineWidth ?? 2)),
+                positions: Cesium.Cartesian3.fromDegreesArray(
+                  geometry.positions.flat(),
+                ),
+                width: Math.min(8, Math.max(1.5, style.lineWidth ?? 2)),
                 material: cesiumColor(style.lineColor, color),
                 clampToGround: true,
               },
             });
-        }
+          } else {
+            const ring = (points) =>
+              Cesium.Cartesian3.fromDegreesArray(points.flat());
+            dataSource.entities.add({
+              id,
+              polygon: {
+                hierarchy: new Cesium.PolygonHierarchy(
+                  ring(geometry.outer),
+                  geometry.holes.map(
+                    (hole) => new Cesium.PolygonHierarchy(ring(hole)),
+                  ),
+                ),
+                material:
+                  style.fill === false
+                    ? Cesium.Color.TRANSPARENT
+                    : cesiumColor(
+                        style.polyColor,
+                        color,
+                        style.polyColor ? undefined : 0.25,
+                      ),
+                classificationType: Cesium.ClassificationType.BOTH,
+              },
+            });
+            if (style.outline !== false)
+              dataSource.entities.add({
+                id: `${id}:outline`,
+                polyline: {
+                  positions: ring([...geometry.outer, geometry.outer[0]]),
+                  width: Math.min(6, Math.max(1.5, style.lineWidth ?? 2)),
+                  material: cesiumColor(style.lineColor, color),
+                  clampToGround: true,
+                },
+              });
+          }
+        });
+        features.push({
+          name: feature.name,
+          folder: feature.folder,
+          description: feature.description,
+          anchor: anchorOf(feature.geometries[0]),
+        });
       });
-      features.push({
-        name: feature.name,
-        folder: feature.folder,
-        description: feature.description,
-        anchor: anchorOf(feature.geometries[0]),
-      });
-    });
-    dataSource.entities.resumeEvents();
-    return { version: product.version, dataSource, urls, features, pins };
+      dataSource.entities.resumeEvents();
+      return { version: product.version, dataSource, urls, features, pins };
+    } catch (error) {
+      urls.forEach((url) => URL.revokeObjectURL(url));
+      throw error;
+    }
   }
 
   /** Bring the globe in line with the catalog and the chip selection. */

src/layers/ghostMaps/lifecycle.test.mjs

@@ -0,0 +1,180 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+import * as Cesium from 'cesium';
+import { createGhostMapsLayer } from './index.js';
+
+function element(localName, children = [], value = '', attributes = {}) {
+  const node = {
+    localName,
+    children,
+    get textContent() {
+      return value + children.map((child) => child.textContent).join('');
+    },
+    getAttribute: (key) => attributes[key] ?? null,
+    getElementsByTagName(name) {
+      return children.flatMap((child) => [
+        ...(name === '*' || name === child.localName ? [child] : []),
+        ...child.getElementsByTagName(name),
+      ]);
+    },
+  };
+  for (const child of children) child.parentElement = node;
+  return node;
+}
+
+function documentFixture() {
+  return element('document', [
+    element('kml', [
+      element('Document', [
+        element(
+          'Style',
+          [
+            element('IconStyle', [
+              element('Icon', [element('href', [], 'icon.png')]),
+            ]),
+          ],
+          '',
+          { id: 'pin' },
+        ),
+        element('Placemark', [
+          element('name', [], 'Test report'),
+          element('styleUrl', [], '#pin'),
+          element('Point', [element('coordinates', [], '12,34')]),
+        ]),
+      ]),
+    ]),
+  ]);
+}
+
+// A stored KMZ with a local icon; the XML DOM double supplies its one point.
+function fixtureKmz() {
+  const files = [
+    ['doc.kml', '<kml/>'],
+    ['icon.png', 'packed icon'],
+  ];
+  const local = [];
+  const central = [];
+  let offset = 0;
+  for (const [name, value] of files) {
+    const key = Buffer.from(name);
+    const bytes = Buffer.from(value);
+    const header = Buffer.alloc(30);
+    header.writeUInt32LE(0x04034b50);
+    header.writeUInt32LE(bytes.length, 18);
+    header.writeUInt32LE(bytes.length, 22);
+    header.writeUInt16LE(key.length, 26);
+    const directory = Buffer.alloc(46);
+    directory.writeUInt32LE(0x02014b50);
+    directory.writeUInt32LE(bytes.length, 20);
+    directory.writeUInt32LE(bytes.length, 24);
+    directory.writeUInt16LE(key.length, 28);
+    directory.writeUInt32LE(offset, 42);
+    local.push(header, key, bytes);
+    central.push(directory, key);
+    offset += header.length + key.length + bytes.length;
+  }
+  const directory = Buffer.concat(central);
+  const end = Buffer.alloc(22);
+  end.writeUInt32LE(0x06054b50);
+  end.writeUInt16LE(files.length, 8);
+  end.writeUInt16LE(files.length, 10);
+  end.writeUInt32LE(directory.length, 12);
+  end.writeUInt32LE(offset, 16);
+  return new Uint8Array(Buffer.concat([...local, directory, end]));
+}
+
+function fixture(t, { beforeFileReturns, onCreateUrl } = {}) {
+  const previous = Object.getOwnPropertyDescriptor(globalThis, 'DOMParser');
+  globalThis.DOMParser = class {
+    parseFromString() {
+      return documentFixture();
+    }
+  };
+  t.after(() => {
+    if (previous) Object.defineProperty(globalThis, 'DOMParser', previous);
+    else delete globalThis.DOMParser;
+  });
+  const created = [];
+  const revoked = [];
+  t.mock.method(URL, 'createObjectURL', () => {
+    const url = `blob:local-${created.length}`;
+    created.push(url);
+    onCreateUrl?.(layer);
+    return url;
+  });
+  t.mock.method(URL, 'revokeObjectURL', (url) => revoked.push(url));
+  t.mock.method(console, 'warn', () => {});
+  const mounted = new Set();
+  const viewer = {
+    scene: { requestRender() {} },
+    dataSources: {
+      async add(dataSource) {
+        mounted.add(dataSource);
+        return dataSource;
+      },
+      remove(dataSource) {
+        return mounted.delete(dataSource);
+      },
+    },
+  };
+  const layer = createGhostMapsLayer({
+    storage: null,
+    source: {
+      getCatalog: async () => ({
+        products: [{ id: 'cip', version: 'a'.repeat(40), defaultOn: true }],
+      }),
+      getFile: async () => {
+        beforeFileReturns?.(layer);
+        return fixtureKmz();
+      },
+    },
+  });
+  layer.init(viewer);
+  layer.enable();
+  t.after(() => layer.destroy());
+  return { layer, viewer, mounted, created, revoked };
+}
+
+test('cancelling a file read does not allocate packed-image URLs', async (t) => {
+  const { layer, viewer, mounted, created, revoked } = fixture(t, {
+    beforeFileReturns: (layer) => queueMicrotask(() => layer.disable()),
+  });
+  await layer.update(viewer);
+  assert.equal(created.length, 0);
+  assert.equal(revoked.length, 0);
+  assert.equal(mounted.size, 0);
+});
+
+test('cancellation during parsing releases every packed-image URL', async (t) => {
+  const { layer, viewer, mounted, created, revoked } = fixture(t, {
+    onCreateUrl: (layer) => layer.disable(),
+  });
+  await layer.update(viewer);
+  assert.equal(created.length, 1);
+  assert.deepEqual(revoked, created);
+  assert.equal(mounted.size, 0);
+});
+
+test('entity construction failure releases parsed image URLs', async (t) => {
+  const { layer, viewer, mounted, created, revoked } = fixture(t);
+  t.mock.method(Cesium.EntityCollection.prototype, 'add', () => {
+    throw new Error('fixture entity failure');
+  });
+  await layer.update(viewer);
+  assert.equal(created.length, 1);
+  assert.deepEqual(revoked, created);
+  assert.equal(mounted.size, 0);
+  assert.equal(layer.getStats().error, 'fixture entity failure');
+});
+
+test('successful load keeps icons until the product is disabled', async (t) => {
+  const { layer, viewer, mounted, created, revoked } = fixture(t);
+  await layer.update(viewer);
+  assert.equal(created.length, 1);
+  assert.equal(revoked.length, 0);
+  assert.equal(mounted.size, 1);
+  assert.equal(layer.getStats().count, 1);
+  layer.disable();
+  assert.deepEqual(revoked, created);
+  assert.equal(mounted.size, 0);
+});