fix: can track and unregister obsolete VSIX themes

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

Author: Akos Kitta

arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts

@@ -238,7 +238,6 @@ import {
   UploadFirmwareDialog,
   UploadFirmwareDialogProps,
 } from './dialogs/firmware-uploader/firmware-uploader-dialog';
-
 import { UploadCertificate } from './contributions/upload-certificate';
 import {
   ArduinoFirmwareUploader,
@@ -332,8 +331,11 @@ import { ThemeServiceWithDB } from './theia/core/theming';
 import { ThemeServiceWithDB as TheiaThemeServiceWithDB } from '@theia/monaco/lib/browser/monaco-indexed-db';
 import {
   MonacoThemingService,
-  ObsoleteThemesCleanup,
+  CleanupObsoleteIndexedDBThemes,
+  ThemesRegistrationSummary,
+  MonacoThemeRegistry,
 } from './theia/monaco/monaco-theming-service';
+import { MonacoThemeRegistry as TheiaMonacoThemeRegistry } from '@theia/monaco/lib/browser/textmate/monaco-theme-registry';
 import { MonacoThemingService as TheiaMonacoThemingService } from '@theia/monaco/lib/browser/monaco-theming-service';
 import { TypeHierarchyServiceProvider } from './theia/typehierarchy/type-hierarchy-service';
 import { TypeHierarchyServiceProvider as TheiaTypeHierarchyServiceProvider } from '@theia/typehierarchy/lib/browser/typehierarchy-service';
@@ -991,8 +993,13 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
 
   // workaround for themes cannot be removed after registration
   // https://github.com/eclipse-theia/theia/issues/11151
-  bind(ObsoleteThemesCleanup).toSelf().inSingletonScope();
-  bind(FrontendApplicationContribution).toService(ObsoleteThemesCleanup);
+  bind(CleanupObsoleteIndexedDBThemes).toSelf().inSingletonScope();
+  bind(FrontendApplicationContribution).toService(
+    CleanupObsoleteIndexedDBThemes
+  );
+  bind(ThemesRegistrationSummary).toSelf().inSingletonScope();
+  bind(MonacoThemeRegistry).toSelf().inSingletonScope();
+  rebind(TheiaMonacoThemeRegistry).toService(MonacoThemeRegistry);
 
   // disable type-hierarchy support
   // https://github.com/eclipse-theia/theia/commit/16c88a584bac37f5cf3cc5eb92ffdaa541bda5be

arduino-ide-extension/src/browser/theia/core/theming.ts

@@ -22,7 +22,7 @@ export namespace ArduinoThemes {
   };
 }
 
-export const builtInThemeIds = new Set(
+const builtInThemeIds = new Set(
   [
     ArduinoThemes.Light,
     ArduinoThemes.Dark,
@@ -35,6 +35,23 @@ export function isBuiltInTheme(theme: Theme | string): boolean {
   return builtInThemeIds.has(themeId);
 }
 
+export function compatibleBuiltInTheme(theme: Theme): Theme {
+  switch (theme.type) {
+    case 'light':
+      return ArduinoThemes.Light;
+    case 'dark':
+      return ArduinoThemes.Dark;
+    case 'hc':
+      return BuiltinThemeProvider.hcTheme;
+    default: {
+      console.warn(
+        `Unhandled theme type: ${theme.type}. Theme ID: ${theme.id}, label: ${theme.label}`
+      );
+      return ArduinoThemes.Light;
+    }
+  }
+}
+
 export const contributedThemeLabel = nls.localize(
   'arduino/theme/contributedTheme',
   'Contributed Theme'

arduino-ide-extension/src/browser/theia/monaco/monaco-theming-service.ts

@@ -1,24 +1,109 @@
+import { CommonCommands } from '@theia/core/lib/browser/common-frontend-contribution';
 import { FrontendApplicationContribution } from '@theia/core/lib/browser/frontend-application';
-import { BuiltinThemeProvider } from '@theia/core/lib/browser/theming';
-import { DisposableCollection } from '@theia/core/lib/common/disposable';
-import URI from '@theia/core/lib/common/uri';
+import { ThemeService } from '@theia/core/lib/browser/theming';
+import { CommandService } from '@theia/core/lib/common/command';
+import {
+  Disposable,
+  DisposableCollection,
+} from '@theia/core/lib/common/disposable';
+import { MessageService } from '@theia/core/lib/common/message-service';
+import { nls } from '@theia/core/lib/common/nls';
+import { deepClone } from '@theia/core/lib/common/objects';
 import { inject, injectable } from '@theia/core/shared/inversify';
 import {
+  MonacoThemeState,
   deleteTheme as deleteThemeFromIndexedDB,
   getThemes as getThemesFromIndexedDB,
 } from '@theia/monaco/lib/browser/monaco-indexed-db';
 import {
   MonacoTheme,
   MonacoThemingService as TheiaMonacoThemingService,
 } from '@theia/monaco/lib/browser/monaco-theming-service';
+import { MonacoThemeRegistry as TheiaMonacoThemeRegistry } from '@theia/monaco/lib/browser/textmate/monaco-theme-registry';
+import type { ThemeMix } from '@theia/monaco/lib/browser/textmate/monaco-theme-types';
 import { HostedPluginSupport } from '@theia/plugin-ext/lib/hosted/browser/hosted-plugin';
-import { ArduinoThemes, builtInThemeIds } from '../core/theming';
+import { ArduinoThemes, compatibleBuiltInTheme } from '../core/theming';
+import { WindowServiceExt } from '../core/window-service-ext';
+
+type MonacoThemeRegistrationSource =
+  /**
+   * When reading JS/TS contributed theme from a JSON file. Such as the Arduino themes and the ones contributed by Theia.
+   */
+  | 'compiled'
+  /**
+   * When reading and registering previous monaco themes from the `indexedDB`.
+   */
+  | 'indexedDB'
+  /**
+   * Contributed by VS Code extensions when starting the app and loading the plugins.
+   */
+  | 'vsix';
+
+@injectable()
+export class ThemesRegistrationSummary {
+  private readonly _summary: Record<MonacoThemeRegistrationSource, string[]> = {
+    compiled: [],
+    indexedDB: [],
+    vsix: [],
+  };
+
+  add(source: MonacoThemeRegistrationSource, themeId: string): void {
+    const themeIds = this._summary[source];
+    if (!themeIds.includes(themeId)) {
+      themeIds.push(themeId);
+    }
+  }
+
+  get summary(): Record<MonacoThemeRegistrationSource, string[]> {
+    return deepClone(this._summary);
+  }
+}
+
+@injectable()
+export class MonacoThemeRegistry extends TheiaMonacoThemeRegistry {
+  @inject(ThemesRegistrationSummary)
+  private readonly summary: ThemesRegistrationSummary;
+
+  private initializing = false;
+
+  override initializeDefaultThemes(): void {
+    this.initializing = true;
+    try {
+      super.initializeDefaultThemes();
+    } finally {
+      this.initializing = false;
+    }
+  }
+
+  override setTheme(name: string, data: ThemeMix): void {
+    super.setTheme(name, data);
+    if (this.initializing) {
+      this.summary.add('compiled', name);
+    }
+  }
+}
 
 @injectable()
 export class MonacoThemingService extends TheiaMonacoThemingService {
-  private readonly _pluginContributedThemeIds = new Set<string>();
+  @inject(ThemesRegistrationSummary)
+  private readonly summary: ThemesRegistrationSummary;
+
+  private themeRegistrationSource: MonacoThemeRegistrationSource | undefined;
+
+  protected override async restore(): Promise<void> {
+    // The custom theme registration must happen before restoring the themes.
+    // Otherwise, theme changes are not picked up.
+    // https://github.com/arduino/arduino-ide/issues/1251#issuecomment-1436737702
+    this.registerArduinoThemes();
+    this.themeRegistrationSource = 'indexedDB';
+    try {
+      await super.restore();
+    } finally {
+      this.themeRegistrationSource = 'indexedDB';
+    }
+  }
 
-  protected override restore(): Promise<void> {
+  private registerArduinoThemes(): void {
     const { Light, Dark } = ArduinoThemes;
     this.registerParsedTheme({
       id: Light.id,
@@ -32,96 +117,124 @@ export class MonacoThemingService extends TheiaMonacoThemingService {
       uiTheme: 'vs-dark',
       json: require('../../../../src/browser/data/dark.color-theme.json'),
     });
-    // The custom theme registration must happen before restoring the themes.
-    // Otherwise, theme changes are not picked up.
-    // https://github.com/arduino/arduino-ide/issues/1251#issuecomment-1436737702
-    return super.restore();
   }
 
-  // Customized to populate `_pluginContributedThemeIds`. The rest of the code is vanilla Theia.
+  protected override doRegisterParsedTheme(
+    state: MonacoThemeState
+  ): Disposable {
+    const themeId = state.id;
+    const source = this.themeRegistrationSource ?? 'compiled';
+    const disposable = super.doRegisterParsedTheme(state);
+    this.summary.add(source, themeId);
+    return disposable;
+  }
+
   protected override async doRegister(
     theme: MonacoTheme,
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     pending: { [uri: string]: Promise<any> },
     toDispose: DisposableCollection
   ): Promise<void> {
     try {
-      const includes = {};
-      const json = await this.loadTheme(
-        theme.uri,
-        includes,
-        pending,
-        toDispose
-      );
-      if (toDispose.disposed) {
-        return;
-      }
-      const label = theme.label || new URI(theme.uri).path.base;
-      const { id, description, uiTheme } = theme;
-      toDispose.push(
-        this.registerParsedTheme({
-          id,
-          label,
-          description,
-          uiTheme: uiTheme,
-          json,
-          includes,
-        })
-      );
-      // This implementation depends on how Theia internally works.
-      // Collect all theme IDs contributed by VSIXs.
-      // When all VISXs are loaded, IDE2 checks the indexedDB for registered themes,
-      // and if there are obsolete ones, deletes them. A theme is obsolete if it was
-      // in the DB but was not loaded from a VSIX during the startup.
-      // See https://github.com/eclipse-theia/theia/issues/11151.
-      if (new URI(theme.uri).scheme === 'file') {
-        this._pluginContributedThemeIds.add(theme.id ?? label);
-      }
-    } catch (e) {
-      console.error('Failed to load theme from ' + theme.uri, e);
+      this.themeRegistrationSource = 'vsix';
+      await super.doRegister(theme, pending, toDispose);
+    } finally {
+      this.themeRegistrationSource = undefined;
     }
   }
-
-  get pluginContributedThemeIds(): string[] {
-    return Array.from(this._pluginContributedThemeIds.values());
-  }
 }
 
-const compiledThemeIds = new Set([
-  ...builtInThemeIds.values(),
-  BuiltinThemeProvider.lightTheme,
-  BuiltinThemeProvider.darkTheme,
-]);
-
 /**
- * Workaround for removing VSIX themes from the indexedDB if they do not load during app startup.
- * This logic cannot be in MonacoThemingService due to a cycle in the DI object graph.
+ * Workaround for removing VSIX themes from the indexedDB if they were not loaded during the app startup.
  */
 @injectable()
-export class ObsoleteThemesCleanup implements FrontendApplicationContribution {
+export class CleanupObsoleteIndexedDBThemes
+  implements FrontendApplicationContribution
+{
   @inject(HostedPluginSupport)
   private readonly hostedPlugin: HostedPluginSupport;
-  @inject(MonacoThemingService)
-  private readonly monacoTheme: MonacoThemingService;
+  @inject(ThemesRegistrationSummary)
+  private readonly summary: ThemesRegistrationSummary;
+  @inject(ThemeService)
+  private readonly themeService: ThemeService;
+  @inject(MessageService)
+  private readonly messageService: MessageService;
+  @inject(CommandService)
+  private readonly commandService: CommandService;
+  @inject(WindowServiceExt)
+  private readonly windowService: WindowServiceExt;
 
   onStart(): void {
-    this.hostedPlugin.didStart.then(() => this.cleanObsoleteThemes());
+    this.hostedPlugin.didStart.then(() => this.cleanupObsoleteThemes());
   }
 
-  private async cleanObsoleteThemes(): Promise<void> {
-    const pluginContributedThemes = this.monacoTheme.pluginContributedThemeIds;
+  private async cleanupObsoleteThemes(): Promise<void> {
     const persistedThemes = await getThemesFromIndexedDB();
-    // if neither registered by code (e.g. webpack load such as the Arduino Theme) or VSIX, remove from the indexedDB.
-    const obsoleteThemes = persistedThemes.filter(
-      ({ id }) =>
-        !pluginContributedThemes.includes(id) && !compiledThemeIds.has(id)
+    const obsoleteThemeIds = collectObsoleteThemeIds(
+      persistedThemes,
+      this.summary.summary
     );
-    if (!obsoleteThemes.length) {
+    if (!obsoleteThemeIds.length) {
       return;
     }
-    await obsoleteThemes.reduce(async (previousTask, theme) => {
+    await obsoleteThemeIds.reduce(async (previousTask, themeId) => {
       await previousTask;
-      return deleteThemeFromIndexedDB(theme.id);
+      return deleteThemeFromIndexedDB(themeId);
     }, Promise.resolve());
+
+    const firstWindow = await this.windowService.isFirstWindow();
+    if (firstWindow) {
+      return this.switchToCompatibleBuiltInTheme(obsoleteThemeIds);
+    }
+  }
+
+  private async switchToCompatibleBuiltInTheme(
+    obsoleteThemeIds: string[]
+  ): Promise<void> {
+    const currentTheme = this.themeService.getCurrentTheme();
+    if (obsoleteThemeIds.includes(currentTheme.id)) {
+      const message = nls.localize(
+        'arduino/theme/couldNotLoadTheme',
+        'Could not load your currently selected theme: {0}. Do you want to automatically select a compatible theme?',
+        currentTheme.label
+      );
+      const yes = nls.localize('vscode/extensionsUtils/yes', 'Yes');
+      const selectManually = nls.localize(
+        'arduino/theme/selectManually',
+        'Select Manually'
+      );
+      const answer = await this.messageService.info(
+        message,
+        selectManually,
+        yes
+      );
+      if (answer === yes) {
+        this.themeService.setCurrentTheme(
+          compatibleBuiltInTheme(currentTheme).id,
+          true
+        );
+      } else if (answer === selectManually) {
+        return this.commandService.executeCommand(
+          CommonCommands.SELECT_COLOR_THEME.id
+        );
+      }
+    }
   }
 }
+
+/**
+ * An indexedDB registered theme is obsolete if it is in the indexedDB but was registered
+ * from neither a `vsix` nor `compiled-json` source during the app startup.
+ */
+export function collectObsoleteThemeIds(
+  indexedDBThemes: MonacoThemeState[],
+  summary: Record<MonacoThemeRegistrationSource, string[]>
+): string[] {
+  const vsixThemeIds = summary['vsix'];
+  const compiledThemeIds = summary['compiled'];
+  return indexedDBThemes
+    .map(({ id }) => id)
+    .filter(
+      (id) => !vsixThemeIds.includes(id) && !compiledThemeIds.includes(id)
+    );
+}

i18n/en.json

@@ -466,6 +466,11 @@
       "dismissSurvey": "Don't show again",
       "surveyMessage": "Please help us improve by answering this super short survey. We value our community and would like to get to know our supporters a little better."
     },
+    "theme": {
+      "contributedTheme": "Contributed Theme",
+      "couldNotLoadTheme": "Could not load your currently selected theme: {0}. Do you want to automatically select a compatible theme?",
+      "selectManually": "Select Manually"
+    },
     "title": {
       "cloud": "Cloud"
     },