fix #5025: reload plugins on reconnect

Also: - fix #5829: ensure that each plugin is loaded only once - fix #6186: start only one plugin host process for the same host instead of a new for each load as before - revert #5257: Reconnect same host plugin process and client" 797db75 Signed-off-by: Anton Kosiakov <anton.kosyakov@typefox.io>
eclipse-theia · Sep 19, 2019 · bfe4bb5 · bfe4bb5
1 parent 2a97874
commit bfe4bb5
Show file tree

Hide file tree

Showing 72 changed files with 1,636 additions and 1,259 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,9 @@ Breaking changes:
 
 - [core][plugin] support alternative commands in context menus [6069](https://github.com/eclipse-theia/theia/pull/6069)
 - [workspace] switched `workspace.supportMultiRootWorkspace` to enabled by default [#6089](https://github.com/eclipse-theia/theia/pull/6089)
+- [core][monaco][plugin] reload plugins on reconnection [6159](https://github.com/eclipse-theia/theia/pull/6159)
+  - Extenders should implement `Disposable` for plugin main services to handle reconnection properly.
+  - Many APIs are refactored to return `Disposable`.
 
 Misc:
 

diff --git a/packages/core/src/browser/keybinding.ts b/packages/core/src/browser/keybinding.ts
@@ -18,6 +18,7 @@ import { injectable, inject, named } from 'inversify';
 import { isOSX } from '../common/os';
 import { Emitter, Event } from '../common/event';
 import { CommandRegistry } from '../common/command';
+import { Disposable, DisposableCollection } from '../common/disposable';
 import { KeyCode, KeySequence, Key } from './keyboard/keys';
 import { KeyboardLayoutService } from './keyboard/keyboard-layout-service';
 import { ContributionProvider } from '../common/contribution-provider';
@@ -184,17 +185,17 @@ export class KeybindingRegistry {
      *
      * @param binding
      */
-    registerKeybinding(binding: Keybinding): void {
-        this.doRegisterKeybinding(binding, KeybindingScope.DEFAULT);
+    registerKeybinding(binding: Keybinding): Disposable {
+        return this.doRegisterKeybinding(binding, KeybindingScope.DEFAULT);
     }
 
     /**
      * Register default keybindings to the registry
      *
      * @param bindings
      */
-    registerKeybindings(...bindings: Keybinding[]): void {
-        this.doRegisterKeybindings(bindings, KeybindingScope.DEFAULT);
+    registerKeybindings(...bindings: Keybinding[]): Disposable {
+        return this.doRegisterKeybindings(bindings, KeybindingScope.DEFAULT);
     }
 
     /**
@@ -222,21 +223,30 @@ export class KeybindingRegistry {
         });
     }
 
-    protected doRegisterKeybindings(bindings: Keybinding[], scope: KeybindingScope = KeybindingScope.DEFAULT): void {
+    protected doRegisterKeybindings(bindings: Keybinding[], scope: KeybindingScope = KeybindingScope.DEFAULT): Disposable {
+        const toDispose = new DisposableCollection();
         for (const binding of bindings) {
-            this.doRegisterKeybinding(binding, scope);
+            toDispose.push(this.doRegisterKeybinding(binding, scope));
         }
+        return toDispose;
     }
 
-    protected doRegisterKeybinding(binding: Keybinding, scope: KeybindingScope = KeybindingScope.DEFAULT): void {
+    protected doRegisterKeybinding(binding: Keybinding, scope: KeybindingScope = KeybindingScope.DEFAULT): Disposable {
         try {
             this.resolveKeybinding(binding);
             if (this.containsKeybinding(this.keymaps[scope], binding)) {
                 throw new Error(`"${binding.keybinding}" is in collision with something else [scope:${scope}]`);
             }
             this.keymaps[scope].push(binding);
+            return Disposable.create(() => {
+                const index = this.keymaps[scope].indexOf(binding);
+                if (index !== -1) {
+                    this.keymaps[scope].splice(index, 1);
+                }
+            });
         } catch (error) {
             this.logger.warn(`Could not register keybinding:\n  ${Keybinding.stringify(binding)}\n${error}`);
+            return Disposable.NULL;
         }
     }
 
@@ -641,16 +651,21 @@ export class KeybindingRegistry {
 
     setKeymap(scope: KeybindingScope, bindings: Keybinding[]): void {
         this.resetKeybindingsForScope(scope);
-        this.doRegisterKeybindings(bindings, scope);
+        this.toResetKeymap.set(scope, this.doRegisterKeybindings(bindings, scope));
         this.keybindingsChanged.fire(undefined);
     }
 
+    protected readonly toResetKeymap = new Map<KeybindingScope, Disposable>();
+
     /**
      * Reset keybindings for a specific scope
      * @param scope scope to reset the keybindings for
      */
     resetKeybindingsForScope(scope: KeybindingScope): void {
-        this.keymaps[scope] = [];
+        const toReset = this.toResetKeymap.get(scope);
+        if (toReset) {
+            toReset.dispose();
+        }
     }
 
     /**

diff --git a/packages/core/src/browser/preferences/preference-contribution.ts b/packages/core/src/browser/preferences/preference-contribution.ts
@@ -16,7 +16,7 @@
 
 import * as Ajv from 'ajv';
 import { inject, injectable, interfaces, named, postConstruct } from 'inversify';
-import { ContributionProvider, bindContributionProvider, escapeRegExpCharacters, Emitter, Event } from '../../common';
+import { ContributionProvider, bindContributionProvider, escapeRegExpCharacters, Emitter, Event, Disposable } from '../../common';
 import { PreferenceScope } from './preference-scope';
 import { PreferenceProvider, PreferenceProviderDataChange } from './preference-provider';
 import {
@@ -26,6 +26,7 @@ import { FrontendApplicationConfigProvider } from '../frontend-application-confi
 import { FrontendApplicationConfig } from '@theia/application-package/lib/application-props';
 import { bindPreferenceConfigurations, PreferenceConfigurations } from './preference-configurations';
 export { PreferenceSchema, PreferenceSchemaProperties, PreferenceDataSchema, PreferenceItem, PreferenceSchemaProperty, PreferenceDataProperty, JsonType };
+import { Mutable } from '../../common/types';
 
 // tslint:disable:no-any
 // tslint:disable:forin
@@ -45,6 +46,12 @@ export interface OverridePreferenceName {
     preferenceName: string
     overrideIdentifier: string
 }
+export namespace OverridePreferenceName {
+    // tslint:disable-next-line:no-any
+    export function is(arg: any): arg is OverridePreferenceName {
+        return !!arg && typeof arg === 'object' && 'preferenceName' in arg && 'overrideIdentifier' in arg;
+    }
+}
 
 const OVERRIDE_PROPERTY = '\\[(.*)\\]$';
 export const OVERRIDE_PROPERTY_PATTERN = new RegExp(OVERRIDE_PROPERTY);
@@ -101,11 +108,12 @@ export class PreferenceSchemaProvider extends PreferenceProvider {
         this.updateOverridePatternPropertiesKey();
     }
 
-    protected readonly overridePatternProperties: Required<Pick<PreferenceDataProperty, 'properties'>> & PreferenceDataProperty = {
+    protected readonly overridePatternProperties: Required<Pick<PreferenceDataProperty, 'properties' | 'additionalProperties'>> & PreferenceDataProperty = {
         type: 'object',
         description: 'Configure editor settings to be overridden for a language.',
         errorMessage: 'Unknown Identifier. Use language identifiers',
-        properties: {}
+        properties: {},
+        additionalProperties: false
     };
     protected overridePatternPropertiesKey: string | undefined;
     protected updateOverridePatternPropertiesKey(): void {
@@ -134,6 +142,32 @@ export class PreferenceSchemaProvider extends PreferenceProvider {
         return param.length ? OVERRIDE_PATTERN_WITH_SUBSTITUTION.replace('${0}', param) : undefined;
     }
 
+    protected doUnsetSchema(changes: PreferenceProviderDataChange[]): PreferenceProviderDataChange[] {
+        const inverseChanges: PreferenceProviderDataChange[] = [];
+        for (const change of changes) {
+            const preferenceName = change.preferenceName;
+            const overridden = this.overriddenPreferenceName(preferenceName);
+            if (overridden) {
+                delete this.overridePatternProperties.properties[`[${overridden.overrideIdentifier}]`];
+                delete this.combinedSchema.properties[`[${overridden.overrideIdentifier}]`];
+            } else {
+                delete this.combinedSchema.properties[preferenceName];
+            }
+            const newValue = change.oldValue;
+            const oldValue = change.newValue;
+            const { scope, domain } = change;
+            const inverseChange: Mutable<PreferenceProviderDataChange> = { preferenceName, oldValue, scope, domain };
+            if (typeof newValue === undefined) {
+                delete this.preferences[preferenceName];
+            } else {
+                inverseChange.newValue = newValue;
+                this.preferences[preferenceName] = newValue;
+            }
+            inverseChanges.push(inverseChange);
+        }
+        return inverseChanges;
+    }
+
     protected doSetSchema(schema: PreferenceSchema): PreferenceProviderDataChange[] {
         const ajv = new Ajv();
         const valid = ajv.validateSchema(schema);
@@ -234,7 +268,9 @@ export class PreferenceSchemaProvider extends PreferenceProvider {
         if (this.configurations.isSectionName(name)) {
             return true;
         }
-        const result = this.validateFunction({ [name]: value }) as boolean;
+        const overridden = this.overriddenPreferenceName(name);
+        const preferenceName = overridden && overridden.preferenceName || name;
+        const result = this.validateFunction({ [preferenceName]: value }) as boolean;
         if (!result && !(name in this.combinedSchema.properties)) {
             // in order to avoid reporting it on each change
             if (!this.unsupportedPreferences.has(name)) {
@@ -249,10 +285,21 @@ export class PreferenceSchemaProvider extends PreferenceProvider {
         return this.combinedSchema;
     }
 
-    setSchema(schema: PreferenceSchema): void {
+    setSchema(schema: PreferenceSchema): Disposable {
         const changes = this.doSetSchema(schema);
+        if (!changes.length) {
+            return Disposable.NULL;
+        }
         this.fireDidPreferenceSchemaChanged();
         this.emitPreferencesChangedEvent(changes);
+        return Disposable.create(() => {
+            const inverseChanges = this.doUnsetSchema(changes);
+            if (!inverseChanges.length) {
+                return;
+            }
+            this.fireDidPreferenceSchemaChanged();
+            this.emitPreferencesChangedEvent(inverseChanges);
+        });
     }
 
     getPreferences(): { [name: string]: any } {
@@ -264,11 +311,24 @@ export class PreferenceSchemaProvider extends PreferenceProvider {
     }
 
     isValidInScope(preferenceName: string, scope: PreferenceScope): boolean {
-        const preference = this.getPreferenceProperty(preferenceName);
-        if (preference) {
-            return preference.scope! >= scope;
+        let property;
+        const overridden = this.overriddenPreferenceName(preferenceName);
+        if (overridden) {
+            // try from overriden schema
+            property = this.overridePatternProperties[`[${overridden.overrideIdentifier}]`];
+            property = property && property[overridden.preferenceName];
+            if (!property) {
+                // try from overriden identifier
+                property = this.overridePatternProperties[overridden.preferenceName];
+            }
+            if (!property) {
+                // try from overriden value
+                property = this.combinedSchema.properties[overridden.preferenceName];
+            }
+        } else {
+            property = this.combinedSchema.properties[preferenceName];
         }
-        return false;
+        return property && property.scope! >= scope;
     }
 
     *getPreferenceNames(): IterableIterator<string> {
@@ -289,11 +349,6 @@ export class PreferenceSchemaProvider extends PreferenceProvider {
         }
     }
 
-    getPreferenceProperty(preferenceName: string): PreferenceItem | undefined {
-        const overridden = this.overriddenPreferenceName(preferenceName);
-        return this.combinedSchema.properties[overridden ? overridden.preferenceName : preferenceName];
-    }
-
     overridePreferenceName({ preferenceName, overrideIdentifier }: OverridePreferenceName): string {
         return `[${overrideIdentifier}].${preferenceName}`;
     }

diff --git a/packages/core/src/browser/preferences/preference-proxy.ts b/packages/core/src/browser/preferences/preference-proxy.ts
@@ -83,12 +83,11 @@ export function createPreferenceProxy<T>(preferences: PreferenceService, schema:
     };
 
     const getValue: PreferenceRetrieval<any>['get'] = (arg, defaultValue, resourceUri) => {
-        const isArgOverridePreferenceName = typeof arg === 'object' && arg.overrideIdentifier;
-        const preferenceName = isArgOverridePreferenceName ?
-            preferences.overridePreferenceName(<OverridePreferenceName>arg) :
+        const preferenceName = OverridePreferenceName.is(arg) ?
+            preferences.overridePreferenceName(arg) :
             <string>arg;
         const value = preferences.get(preferenceName, defaultValue, resourceUri || opts.resourceUri);
-        if (preferences.validate(isArgOverridePreferenceName ? (<OverridePreferenceName>arg).preferenceName : preferenceName, value)) {
+        if (preferences.validate(preferenceName, value)) {
             return value;
         }
         if (defaultValue !== undefined) {

diff --git a/packages/core/src/browser/preferences/preference-service.spec.ts b/packages/core/src/browser/preferences/preference-service.spec.ts
@@ -183,6 +183,68 @@ describe('Preference Service', () => {
         expect(prefService.get('test.number')).equals(0);
     });
 
+    it('should unset preference schema', () => {
+        const events: PreferenceChange[] = [];
+        prefService.onPreferenceChanged(event => events.push(event));
+
+        prefSchema.registerOverrideIdentifier('go');
+
+        const toUnset = prefSchema.setSchema({
+            properties: {
+                'editor.insertSpaces': {
+                    type: 'boolean',
+                    default: true,
+                    overridable: true
+                },
+                '[go]': {
+                    type: 'object',
+                    default: {
+                        'editor.insertSpaces': false
+                    }
+                }
+            }
+        });
+
+        assert.deepStrictEqual([{
+            preferenceName: 'editor.insertSpaces',
+            newValue: true,
+            oldValue: undefined
+        }, {
+            preferenceName: '[go].editor.insertSpaces',
+            newValue: false,
+            oldValue: undefined
+        }], events.map(e => ({
+            preferenceName: e.preferenceName,
+            newValue: e.newValue,
+            oldValue: e.oldValue
+        })), 'events before');
+        assert.strictEqual(prefService.get('editor.insertSpaces'), true, 'get before');
+        assert.strictEqual(prefService.get('[go].editor.insertSpaces'), false, 'get before overridden');
+        assert.strictEqual(prefSchema.validate('editor.insertSpaces', false), true, 'validate before');
+        assert.strictEqual(prefSchema.validate('[go].editor.insertSpaces', true), true, 'validate before overridden');
+
+        events.length = 0;
+        toUnset.dispose();
+
+        assert.deepStrictEqual([{
+            preferenceName: 'editor.insertSpaces',
+            newValue: undefined,
+            oldValue: true
+        }, {
+            preferenceName: '[go].editor.insertSpaces',
+            newValue: undefined,
+            oldValue: false
+        }], events.map(e => ({
+            preferenceName: e.preferenceName,
+            newValue: e.newValue,
+            oldValue: e.oldValue
+        })), 'events after');
+        assert.strictEqual(prefService.get('editor.insertSpaces'), undefined, 'get after');
+        assert.strictEqual(prefService.get('[go].editor.insertSpaces'), undefined, 'get after overridden');
+        assert.strictEqual(prefSchema.validate('editor.insertSpaces', true), false, 'validate after');
+        assert.strictEqual(prefSchema.validate('[go].editor.insertSpaces', true), false, 'validate after overridden');
+    });
+
     describe('overridden preferences', () => {
 
         it('get #0', () => {

diff --git a/packages/core/src/browser/preferences/preference-service.ts b/packages/core/src/browser/preferences/preference-service.ts
@@ -195,6 +195,10 @@ export class PreferenceServiceImpl implements PreferenceService {
                             acceptChange(change);
                         }
                     }
+                } else if (change.newValue === undefined && change.scope === PreferenceScope.Default) {
+                    // preference is removed
+                    acceptChange(change);
+                    break;
                 }
             }
         }

diff --git a/packages/core/src/browser/quick-open/quick-command-service.ts b/packages/core/src/browser/quick-open/quick-command-service.ts
@@ -15,7 +15,7 @@
  ********************************************************************************/
 
 import { inject, injectable } from 'inversify';
-import { Command, CommandRegistry } from '../../common';
+import { Command, CommandRegistry, Disposable } from '../../common';
 import { Keybinding, KeybindingRegistry } from '../keybinding';
 import { QuickOpenModel, QuickOpenItem, QuickOpenMode, QuickOpenGroupItem, QuickOpenGroupItemOptions } from './quick-open-model';
 import { QuickOpenOptions } from './quick-open-service';
@@ -51,10 +51,16 @@ export class QuickCommandService implements QuickOpenModel, QuickOpenHandler {
     protected readonly corePreferences: CorePreferences;
 
     protected readonly contexts = new Map<string, string[]>();
-    pushCommandContext(commandId: string, when: string): void {
+    pushCommandContext(commandId: string, when: string): Disposable {
         const contexts = this.contexts.get(commandId) || [];
         contexts.push(when);
         this.contexts.set(commandId, contexts);
+        return Disposable.create(() => {
+            const index = contexts.indexOf(when);
+            if (index !== -1) {
+                contexts.splice(index, 1);
+            }
+        });
     }
 
     /** Initialize this quick open model with the commands. */

diff --git a/packages/core/src/browser/shell/tab-bars.ts b/packages/core/src/browser/shell/tab-bars.ts
@@ -222,6 +222,13 @@ export class TabBarRenderer extends TabBar.Renderer {
      */
     protected getDecorations(title: Title<Widget>): WidgetDecoration.Data[] {
         if (this.tabBar && this.decoratorService) {
+            // tslint:disable-next-line:no-any
+            const owner: { resetTabBarDecorations?: () => void; } & Widget = title.owner;
+            if (!owner.resetTabBarDecorations) {
+                owner.resetTabBarDecorations = () => this.decorations.delete(title);
+                title.owner.disposed.connect(owner.resetTabBarDecorations);
+            }
+
             const decorations = this.decorations.get(title) || this.decoratorService.getDecorations(title);
             this.decorations.set(title, decorations);
             return decorations;