HARP-12599: Remove access of deprecated MapView.theme accessors and f…

…ix resulting errors (#1928)

* Fixes getTheme Promise to not resolve between loading and updating
* Removes usage of deprecated MapView.theme accessors

Signed-off-by: Frauke Fritz <frauke.fritz@here.com>

@here/harp-examples/src/datasource_here-webtile.ts

@@ -78,8 +78,10 @@ export namespace HereWebTileDataSourceExample {
 
         // Add an UI.
         const ui = new MapControlsUI(controls, { zoomLevel: "input", projectionSwitch: true });
-        ui.projectionSwitchElement?.addEventListener("click", () => {
-            map.theme = map.projection.type === ProjectionType.Spherical ? GLOBE_THEME : FLAT_THEME;
+        ui.projectionSwitchElement?.addEventListener("click", async () => {
+            await map.setTheme(
+                map.projection.type === ProjectionType.Spherical ? GLOBE_THEME : FLAT_THEME
+            );
         });
         canvas.parentElement!.appendChild(ui.domElement);
 

@here/harp-examples/src/getting-started_open-sourced-themes.ts

@@ -3,10 +3,9 @@
  * Licensed under Apache 2.0, see full license in LICENSE
  * SPDX-License-Identifier: Apache-2.0
  */
-
 import { GeoCoordinates } from "@here/harp-geoutils";
 import { MapControls, MapControlsUI } from "@here/harp-map-controls";
-import { CopyrightElementHandler, MapView, ThemeLoader } from "@here/harp-mapview";
+import { CopyrightElementHandler, MapView } from "@here/harp-mapview";
 import { VectorTileDataSource } from "@here/harp-vectortile-datasource";
 import { GUI } from "dat.gui";
 
@@ -65,10 +64,8 @@ export namespace ThemesExample {
         }
     };
     gui.add(options, "theme", options.theme)
-        .onChange((value: string) => {
-            ThemeLoader.load(value).then(theme => {
-                mapView.setTheme(theme);
-            });
+        .onChange(async (value: string) => {
+            await mapView.setTheme(value);
         })
         .setValue("resources/berlin_tilezen_base.json");
 }

@here/harp-examples/src/performance_benchmark.ts

@@ -740,9 +740,9 @@ export namespace PerformanceBenchmark {
                 };
 
                 ThemeLoader.load(themeUrl, { uriResolver: fontUriResolver }).then(
-                    (newTheme: Theme) => {
+                    async (newTheme: Theme) => {
                         mapViewApp.mapView.clearTileCache();
-                        mapViewApp.mapView.setTheme(newTheme);
+                        await mapViewApp.mapView.setTheme(newTheme);
                     }
                 );
             })

@here/harp-examples/src/rendering_post-effects_themes.ts

@@ -76,9 +76,8 @@ export namespace EffectsExample {
     };
     const selector = gui.add(options, "theme", options.theme);
     selector
-        .onChange((value: string) => {
-            map.clearTileCache();
-            map.setTheme(value);
+        .onChange(async (value: string) => {
+            await map.setTheme(value);
             map.loadPostEffects((options.postEffects as { [key: string]: string })[value]);
         })
         .setValue(options.theme.streets);

@here/harp-mapview/lib/DataSource.ts

@@ -323,9 +323,10 @@ export abstract class DataSource extends THREE.EventDispatcher {
      * {@link MapView}s theme.
      */
     set styleSetName(styleSetName: string | undefined) {
-        this.m_styleSetName = styleSetName;
-        if (this.m_mapView !== undefined && styleSetName !== undefined) {
-            this.setTheme(this.m_mapView.theme);
+        if (styleSetName !== this.m_styleSetName) {
+            this.m_styleSetName = styleSetName;
+            this.clearCache();
+            this.requestUpdate();
         }
     }
 

@here/harp-mapview/lib/MapView.ts

@@ -3507,7 +3507,6 @@ export class MapView extends EventDispatcher {
             !this.m_initialTextPlacementDone &&
             !this.m_firstFrameComplete &&
             !this.isDynamicFrame &&
-            !this.m_themeManager.isLoading() &&
             !this.m_themeManager.isUpdating() &&
             this.m_poiTableManager.finishedLoading &&
             this.m_visibleTiles.allVisibleTilesLoaded &&
@@ -3915,8 +3914,10 @@ export class MapView extends EventDispatcher {
     private readonly onWebGLContextRestored = (event: Event) => {
         this.dispatchEvent(this.CONTEXT_RESTORED_EVENT);
         if (this.m_renderer !== undefined) {
-            this.m_sceneEnvironment.updateClearColor(this.theme);
-            this.update();
+            this.getTheme().then(theme => {
+                this.m_sceneEnvironment.updateClearColor(theme);
+                this.update();
+            });
         }
         logger.warn("WebGL context restored", event);
     };

@here/harp-mapview/lib/MapViewThemeManager.ts

@@ -17,29 +17,37 @@ const logger = LoggerManager.instance.create("MapViewThemeManager");
  */
 export class MapViewThemeManager {
     private readonly m_imageCache: MapViewImageCache;
-    private m_themePromise: Promise<Theme> | undefined;
+    private m_updatePromise: Promise<void> | undefined;
     private m_abortControllers: AbortController[] = [];
     private m_theme: Theme = {};
-    private m_isUpdating: boolean = false;
 
     constructor(private readonly m_mapView: MapView, private readonly m_uriResolver?: UriResolver) {
         this.m_imageCache = new MapViewImageCache(this.m_mapView);
     }
 
-    async getTheme(): Promise<Theme> {
-        if (!this.m_themePromise) {
-            return this.m_theme;
-        } else {
-            return await this.m_themePromise;
+    async setTheme(theme: Theme | string): Promise<Theme> {
+        if (this.isUpdating()) {
+            logger.warn("Formerly set Theme is still updating, update will be canceled");
+            this.cancelThemeUpdate();
         }
+
+        this.m_updatePromise = this.loadTheme(theme).then(async theme => {
+            await this.updateTheme(theme);
+        });
+        await this.m_updatePromise;
+        this.m_updatePromise = undefined;
+        return this.m_theme;
     }
 
-    isLoading(): boolean {
-        return this.m_themePromise !== undefined;
+    async getTheme(): Promise<Theme> {
+        if (this.isUpdating()) {
+            await this.m_updatePromise;
+        }
+        return this.m_theme;
     }
 
     isUpdating(): boolean {
-        return this.m_isUpdating;
+        return this.m_updatePromise !== undefined;
     }
 
     /**
@@ -48,36 +56,29 @@ export class MapViewThemeManager {
      * after deprecation
      */
     get theme() {
-        return this.isLoading() ? {} : this.m_theme;
+        return this.isUpdating() ? {} : this.m_theme;
     }
 
     private async loadTheme(theme: Theme | string): Promise<Theme> {
         if (typeof theme === "string" || !ThemeLoader.isThemeLoaded(theme)) {
             try {
-                this.m_themePromise = ThemeLoader.load(theme, {
+                theme = await ThemeLoader.load(theme, {
                     uriResolver: this.m_uriResolver,
                     signal: this.createAbortController().signal
                 });
-                theme = await this.m_themePromise;
             } catch (error) {
-                logger.error(`failed to load theme: ${error}`, error);
+                if (error.name === "AbortError") {
+                    logger.warn(`theme loading was aborted due to: ${error}`);
+                } else {
+                    logger.error(`failed to load theme: ${error}`);
+                }
                 theme = {};
             }
         }
-        this.m_themePromise = undefined;
         return theme;
     }
 
-    async setTheme(theme: Theme | string): Promise<Theme> {
-        if (this.isLoading() || this.isUpdating()) {
-            logger.warn("Formerly set Theme is still updating");
-            this.m_themePromise = undefined;
-            this.cancelThemeUpdate();
-        }
-
-        theme = await this.loadTheme(theme);
-
-        this.m_isUpdating = true;
+    private async updateTheme(theme: Theme): Promise<void> {
         const environment = this.m_mapView.sceneEnvironment;
         // Fog and sky.
         this.m_theme.fog = theme.fog;
@@ -92,15 +93,14 @@ export class MapViewThemeManager {
         this.m_theme.clearColor = theme.clearColor;
         this.m_theme.clearAlpha = theme.clearAlpha;
         environment.updateClearColor(theme);
+
         // Images.
         this.m_theme.images = theme.images;
         this.m_theme.imageTextures = theme.imageTextures;
         await this.updateImages(theme);
-
         // POI tables.
         this.m_theme.poiTables = theme.poiTables;
         await this.loadPoiTables(theme);
-
         // Text.
         this.m_theme.textStyles = theme.textStyles;
         this.m_theme.defaultTextStyle = theme.defaultTextStyle;
@@ -125,11 +125,10 @@ export class MapViewThemeManager {
         this.m_theme.styles = theme.styles ?? {};
         this.m_theme.definitions = theme.definitions;
 
+        // TODO: this is asynchronouse too
         for (const dataSource of this.m_mapView.dataSources) {
             dataSource.setTheme(this.m_theme);
         }
-        this.m_isUpdating = false;
-        return this.m_theme;
     }
 
     updateCache() {

@here/harp-mapview/test/DataSourceTest.ts

@@ -113,7 +113,7 @@ describe("DataSource", function() {
                 dataSource.attach(mapView);
                 dataSource.styleSetName = "test";
                 expect(dataSource.styleSetName).to.equal("test");
-                expect(setThemeSpy.calledOnceWith(mapView.theme));
+                expect(setThemeSpy.calledOnce);
             });
         });
 

@here/harp-mapview/test/MapViewTest.ts

@@ -643,7 +643,6 @@ describe("MapView", function() {
 
     it("Correctly sets event listeners and handlers webgl context restored", async function() {
         mapView = new MapView({ canvas });
-        const updateSpy = sinon.spy(mapView, "update");
 
         expect(addEventListenerSpy.callCount).to.be.equal(2);
         expect(addEventListenerSpy.callCount).to.be.equal(2);
@@ -655,41 +654,25 @@ describe("MapView", function() {
         const webGlContextRestoredHandler = addEventListenerSpy.getCall(1).args[1];
         const webGlContextLostHandler = addEventListenerSpy.getCall(0).args[1];
 
-        const dispatchEventSpy = sinon.spy(mapView, "dispatchEvent");
         await mapView.setTheme({
             clearColor: "#ffffff"
         });
-        webGlContextRestoredHandler();
+        expect(clearColorStub.calledWith("#ffffff"));
+
+        await webGlContextRestoredHandler();
+        expect(clearColorStub.calledWith(DEFAULT_CLEAR_COLOR));
 
         await mapView.setTheme({
             clearColor: undefined
         });
-        webGlContextRestoredHandler();
-        webGlContextLostHandler();
 
-        expect(clearColorStub.callCount).to.be.equal(6);
-        expect(clearColorStub.getCall(0).calledWith(DEFAULT_CLEAR_COLOR)).to.be.equal(true);
-        expect(clearColorStub.getCall(2).args[0].r).to.be.equal(1);
-        expect(clearColorStub.getCall(2).args[0].g).to.be.equal(1);
-        expect(clearColorStub.getCall(2).args[0].b).to.be.equal(1);
-        expect(clearColorStub.getCall(4).calledWith(DEFAULT_CLEAR_COLOR)).to.be.equal(true);
-
-        expect(updateSpy.callCount).to.be.equal(6);
-        expect(dispatchEventSpy.callCount).to.be.equal(13);
-        expect(dispatchEventSpy.getCall(6).args[0].type).to.be.equal(
-            MapViewEventNames.ContextRestored
-        );
+        expect(clearColorStub.calledWith(DEFAULT_CLEAR_COLOR));
 
-        expect(dispatchEventSpy.getCall(7).args[0].type).to.be.equal(MapViewEventNames.Update);
-        expect(dispatchEventSpy.getCall(8).args[0].type).to.be.equal(MapViewEventNames.ThemeLoaded);
-        expect(dispatchEventSpy.getCall(9).args[0].type).to.be.equal(MapViewEventNames.Update);
-        expect(dispatchEventSpy.getCall(10).args[0].type).to.be.equal(
-            MapViewEventNames.ContextRestored
-        );
-        expect(dispatchEventSpy.getCall(11).args[0].type).to.be.equal(MapViewEventNames.Update);
-        expect(dispatchEventSpy.getCall(12).args[0].type).to.be.equal(
-            MapViewEventNames.ContextLost
-        );
+        await webGlContextRestoredHandler();
+        expect(clearColorStub.calledWith(DEFAULT_CLEAR_COLOR));
+
+        webGlContextLostHandler();
+        expect(clearColorStub.calledWith(DEFAULT_CLEAR_COLOR));
     });
 
     it("Correctly sets and removes all event listeners by API", function() {
@@ -1464,8 +1447,7 @@ describe("MapView", function() {
                 clearTimeout(id);
             };
         }
-        mapView = new MapView({ canvas });
-        mapView.theme = {};
+        mapView = new MapView({ canvas, theme: {} });
 
         const dataSource = new FakeOmvDataSource({ name: "omv" });
 
@@ -1648,7 +1630,7 @@ describe("MapView", function() {
         it("loads a default theme", async function() {
             mapView = new MapView({ canvas });
             await waitForEvent(mapView, MapViewEventNames.ThemeLoaded);
-            expect(mapView.theme).to.deep.equal({
+            expect(await mapView.getTheme()).to.deep.equal({
                 clearAlpha: undefined,
                 clearColor: undefined,
                 defaultTextStyle: undefined,
@@ -1672,9 +1654,9 @@ describe("MapView", function() {
                 canvas,
                 theme: relativeToAppUrl
             });
-            await waitForEvent(mapView, MapViewEventNames.ThemeLoaded);
+            const theme = await mapView.getTheme();
 
-            expect(mapView.theme.styles).to.not.be.empty;
+            expect(theme.styles).to.not.be.empty;
         });
 
         it("allows to reset theme", async function() {
@@ -1686,7 +1668,7 @@ describe("MapView", function() {
             });
             await waitForEvent(mapView, MapViewEventNames.ThemeLoaded);
 
-            expect(mapView.theme).to.not.deep.equal({
+            expect(mapView.getTheme()).to.not.deep.equal({
                 clearColor: undefined,
                 defaultTextStyle: undefined,
                 definitions: undefined,
@@ -1701,7 +1683,7 @@ describe("MapView", function() {
                 textStyles: undefined
             });
 
-            mapView.theme = {};
+            await mapView.setTheme({});
             mapView.getTheme().then(theme =>
                 expect(theme).to.deep.equal({
                     clearAlpha: undefined,

@here/harp-mapview/test/TileGeometryCreatorTest.ts

@@ -378,10 +378,6 @@ describe("TileGeometryCreator", () => {
             }
         ];
 
-        newTile.mapView.theme = {
-            styles: { rules }
-        };
-
         // create `StyleSetEvaluator` to instantiate techniques
         // for the test polygons.
         const styleSetEvaluator = new StyleSetEvaluator({ styleSet: rules });