HARP-8560 Fix for missing tile data bug.

* Don't remove tile loaders from cache that are still in use.
* Refactoring in TransferManager.
* Added more unit tests.

Signed-off-by: Marcel Pursche <marcel.pursche@here.com>

@here/harp-mapview-decoder/lib/TileDataSource.ts

@@ -175,6 +175,9 @@ export class TileDataSource<TileType extends Tile> extends DataSource {
             // Cancel any pending downloads as early as possible.
             tileLoader.cancel();
         };
+        this.m_tileLoaderCache.canEvict = (_, tileLoader) => {
+            return tileLoader.frameNumLastUsed < this.mapView.frameNumber;
+        };
     }
 
     /** @override */
@@ -261,14 +264,16 @@ export class TileDataSource<TileType extends Tile> extends DataSource {
         const mortonCode = tileKey.mortonCode();
         const tileLoader = this.m_tileLoaderCache.get(mortonCode);
         if (tileLoader !== undefined) {
+            tileLoader.frameNumLastUsed = this.mapView.frameNumber;
             tile.tileLoader = tileLoader;
         } else {
             const newTileLoader = new TileLoader(
                 this,
                 tileKey,
                 this.m_options.dataProvider,
                 this.decoder,
-                0
+                0,
+                this.mapView.frameNumber
             );
             tile.tileLoader = newTileLoader;
             tile.copyrightInfo = this.m_options.copyrightInfo;

@here/harp-mapview-decoder/lib/TileLoader.ts

@@ -87,13 +87,15 @@ export class TileLoader {
      * @param dataProvider The [[DataProvider]] that retrieves the binary tile data.
      * @param tileDecoder The [[ITileDecoder]] that decodes the binary tile to a [[DecodeTile]].
      * @param priority The priority given to the loading job. Highest number will be served first.
+     * @param frameNumLastUsed The frame number the tile loader was last used.
      */
     constructor(
         protected dataSource: DataSource,
         protected tileKey: TileKey,
         protected dataProvider: DataProvider,
         protected tileDecoder: ITileDecoder,
-        public priority: number
+        public priority: number,
+        public frameNumLastUsed: number = -1
     ) {}
 
     /**

@here/harp-mapview-decoder/test/TileDataSourceTest.ts

@@ -79,7 +79,8 @@ function createMockMapView() {
         projection: webMercatorProjection,
         // tslint:disable-next-line:no-empty
         getDataSourceByName() {},
-        statistics: new Statistics()
+        statistics: new Statistics(),
+        frameNumber: 0
     } as any) as MapView;
 }
 
@@ -386,4 +387,52 @@ describe("TileDataSource", function() {
         // assert
         assert.equal(tile.hasGeometry, true);
     });
+
+    it("Currently used tile loaders aren't canceled", async function() {
+        const mockedDataSource = getDataSource();
+        const { testedDataSource } = mockedDataSource;
+        const numTiles = (testedDataSource as any).getCacheCount() + 16;
+
+        // Request more tile loaders that fit into tile loader cache
+        const tiles: Tile[] = [];
+        for (let i = 0; i < numTiles; ++i) {
+            const tile = testedDataSource.getTile(TileKey.fromMortonCode(i));
+            tiles.push(tile!);
+        }
+
+        // Check that no tile loader was canceled
+        for (const tile of tiles) {
+            assert.notEqual(tile.tileLoader!.state, TileLoaderState.Canceled);
+
+            await tile.tileLoader!.waitSettled();
+            assert(tile.tileLoader!.isFinished);
+        }
+    });
+
+    it("Cancels tile loaders evicted from cache", async function() {
+        const mockedDataSource = getDataSource();
+        const { testedDataSource } = mockedDataSource;
+        const mapView = testedDataSource.mapView;
+        const numTiles = (testedDataSource as any).getCacheCount() + 16;
+
+        // Request that should be canceled
+        const tileToCancel = testedDataSource.getTile(TileKey.fromMortonCode(0))!;
+
+        // Request more tiles for next frame
+        (mapView as any).frameNumber = 1;
+        const tiles: Tile[] = [];
+        for (let i = 1; i < numTiles; ++i) {
+            const tile = testedDataSource.getTile(TileKey.fromMortonCode(i));
+            tiles.push(tile!);
+        }
+
+        // Check that only tile loader used in previous frame was canceled
+        assert.equal(tileToCancel.tileLoader!.state, TileLoaderState.Canceled);
+        for (const tile of tiles) {
+            assert.notEqual(tile.tileLoader!.state, TileLoaderState.Canceled);
+
+            await tile.tileLoader!.waitSettled();
+            assert(tile.tileLoader!.isFinished);
+        }
+    });
 });

@here/harp-transfer-manager/package.json

@@ -28,9 +28,11 @@
     },
     "devDependencies": {
         "@types/chai": "^4.1.2",
+        "@types/chai-as-promised": "^7.1.2",
         "@types/mocha": "^5.2.7",
         "@types/sinon": "^7.0.13",
         "chai": "^4.0.2",
+        "chai-as-promised": "^7.1.1",
         "cross-env": "^6.0.3",
         "glob": "^7.1.1",
         "mocha": "^6.1.4",

@here/harp-transfer-manager/src/DeferredPromise.ts

@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2017-2019 HERE Europe B.V.
+ * Licensed under Apache 2.0, see full license in LICENSE
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/**
+ *
+ * DeferredPromise takes an executor function for executing it later, when [[exec]] is called.
+ * This class allows wrapping other promises or long running functions for later execution.
+ * @internal
+ */
+export class DeferredPromise<T> {
+    /**
+     * Internal promise to store the result of the deferred executor function.
+     */
+    readonly promise: Promise<T>;
+    private resolveFunc?: (result?: T) => void;
+    private rejectFunc?: (reason?: any) => void;
+
+    /**
+     * Constructs a new [[DeferredPromise]]
+     * @param executor Async function that should be executed at a later point in time.
+     */
+    constructor(private readonly executor: () => Promise<T>) {
+        this.promise = new Promise<T>((resolve, reject) => {
+            this.resolveFunc = resolve;
+            this.rejectFunc = reject;
+        });
+    }
+
+    /**
+     * When `exec` is called the deferred executor function is executed.
+     */
+    exec() {
+        this.executor()
+            .then(result => this.resolveFunc!(result))
+            .catch(error => this.rejectFunc!(error));
+    }
+}

@here/harp-transfer-manager/src/TransferManager.ts

@@ -11,53 +11,7 @@
  */
 
 import "@here/harp-fetch";
-
-/** @internal
- * DeferredPromise takes an executor function for executing it later, when [[exec]] is called.
- * This class allows wrapping other promises or long running functions for later execution.
- */
-class DeferredPromise<T> {
-    /**
-     * Internal promise to store the deferred executor function.
-     */
-    readonly promise: Promise<T>;
-    private doExec = false;
-    private resolveFunc?: (result?: T) => void;
-    private rejectFunc?: (reason?: any) => void;
-
-    constructor(private readonly executor: () => Promise<T>) {
-        this.promise = new Promise<T>((resolve, reject) => {
-            this.resolveFunc = resolve;
-            this.rejectFunc = reject;
-
-            if (this.doExec) {
-                this.execInnerPromise(this.resolveFunc, this.rejectFunc);
-            }
-        });
-    }
-
-    /**
-     * When `exec` is called the deferred executor function is executed.
-     */
-    exec() {
-        if (this.resolveFunc === undefined || this.rejectFunc === undefined) {
-            // deferred promise not yet initialized - handle it in callback above
-            this.doExec = true;
-            return;
-        }
-
-        this.execInnerPromise(this.resolveFunc, this.rejectFunc);
-    }
-
-    private execInnerPromise(
-        resolveFunc: (result?: T) => void,
-        rejectFunc: (reason?: any) => void
-    ) {
-        this.executor()
-            .then(result => resolveFunc(result))
-            .catch(err => rejectFunc(err));
-    }
-}
+import { DeferredPromise } from "./DeferredPromise";
 
 /**
  * `TransferManager` for downloading URLs.
@@ -170,17 +124,23 @@ export class TransferManager {
         }
         return this.doDownload(url, init);
     }
-    private doDownload(url: string, init?: RequestInit): Promise<Response> {
-        ++this.activeDownloadCount;
-        return TransferManager.fetchRepeatedly(this.fetchFunction, 0, this.maxRetries, url, init)
-            .then(response => {
-                this.onDownloadDone();
-                return response;
-            })
-            .catch(err => {
-                this.onDownloadDone();
-                throw err;
-            });
+    private async doDownload(url: string, init?: RequestInit): Promise<Response> {
+        try {
+            ++this.activeDownloadCount;
+            const response = await TransferManager.fetchRepeatedly(
+                this.fetchFunction,
+                0,
+                this.maxRetries,
+                url,
+                init
+            );
+
+            this.onDownloadDone();
+            return response;
+        } catch (error) {
+            this.onDownloadDone();
+            throw error;
+        }
     }
     private onDownloadDone() {
         --this.activeDownloadCount;

@here/harp-transfer-manager/test/DeferredPromiseTest.ts

@@ -0,0 +1,68 @@
+/*
+ * Copyright (C) 2017-2019 HERE Europe B.V.
+ * Licensed under Apache 2.0, see full license in LICENSE
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+// tslint:disable:completed-docs
+// tslint:disable:only-arrow-functions
+//    Mocha discourages using arrow functions, see https://mochajs.org/#arrow-functions
+
+import * as chai from "chai";
+import * as chaiAsPromised from "chai-as-promised";
+chai.use(chaiAsPromised);
+const { expect } = chai;
+
+import { DeferredPromise } from "../src/DeferredPromise";
+
+function delay(timeout: number) {
+    return new Promise(resolve => {
+        setTimeout(resolve, timeout);
+    });
+}
+
+describe("DeferredPromise", function() {
+    it("does not settle Promise unless exec() was called.", async function() {
+        let isSettled = false;
+        const deferred = new DeferredPromise(() => {
+            isSettled = true;
+            return Promise.resolve();
+        });
+
+        // Delay execution
+        await delay(1);
+
+        // tslint:disable-next-line: no-unused-expression
+        expect(deferred.promise).to.not.be.undefined;
+        // tslint:disable-next-line: no-unused-expression
+        expect(isSettled).to.be.false;
+    });
+
+    it("settles a Promise when exec() is called.", async function() {
+        let isSettled = false;
+        const deferred = new DeferredPromise(() => {
+            isSettled = true;
+            return Promise.resolve();
+        });
+
+        deferred.exec();
+        await expect(deferred.promise).to.eventually.be.fulfilled;
+
+        // tslint:disable-next-line: no-unused-expression
+        expect(isSettled).to.be.true;
+    });
+
+    it("handles a rejected Promise.", async function() {
+        let isSettled = false;
+        const deferred = new DeferredPromise(() => {
+            isSettled = true;
+            return Promise.reject("Some error happened.");
+        });
+
+        deferred.exec();
+        await expect(deferred.promise).to.eventually.be.rejectedWith("Some error happened.");
+
+        // tslint:disable-next-line: no-unused-expression
+        expect(isSettled).to.be.true;
+    });
+});