eclipse-theia#5093: Avoid call of unsupported function on Proxy

Extracted KeyValidator interface to be able to detect whether that service is supported. Signed-off-by: Miro Spönemann <miro.spoenemann@typefox.io>
fangnx · May 8, 2019 · 54e4e1d · 54e4e1d
1 parent ca01592
commit 54e4e1d
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 42 deletions.
diff --git a/packages/core/src/browser/keyboard/browser-keyboard-layout-provider.spec.ts b/packages/core/src/browser/keyboard/browser-keyboard-layout-provider.spec.ts
@@ -24,7 +24,7 @@ import { LocalStorageService } from '../storage-service';
 import { MessageService } from '../../common/message-service';
 import { WindowService } from '../window/window-service';
 import { BrowserKeyboardLayoutProvider } from './browser-keyboard-layout-provider';
-import { Key, KeyCode } from './keys';
+import { Key } from './keys';
 
 describe('browser keyboard layout provider', function () {
 
@@ -72,7 +72,7 @@ describe('browser keyboard layout provider', function () {
         });
 
         chai.expect((currentLayout.info as IMacKeyboardLayoutInfo).id).to.equal('com.apple.keylayout.US');
-        service.validateKeyCode(new KeyCode({ key: Key.SEMICOLON, character: 'ö' }));
+        service.validateKey({ code: Key.SEMICOLON.code, character: 'ö' });
         chai.expect((currentLayout.info as IMacKeyboardLayoutInfo).id).to.equal('com.apple.keylayout.German');
     });
 
@@ -84,7 +84,7 @@ describe('browser keyboard layout provider', function () {
         });
 
         chai.expect((currentLayout.info as IMacKeyboardLayoutInfo).id).to.equal('com.apple.keylayout.US');
-        service.validateKeyCode(new KeyCode({ key: Key.SEMICOLON, character: 'm' }));
+        service.validateKey({ code: Key.SEMICOLON.code, character: 'm' });
         chai.expect((currentLayout.info as IMacKeyboardLayoutInfo).id).to.equal('com.apple.keylayout.French');
     });
 
@@ -95,11 +95,11 @@ describe('browser keyboard layout provider', function () {
             currentLayout = l;
         });
 
-        service.validateKeyCode(new KeyCode({ key: Key.QUOTE, character: 'ä' }));
-        service.validateKeyCode(new KeyCode({ key: Key.SEMICOLON, character: 'ö' }));
-        service.validateKeyCode(new KeyCode({ key: Key.BRACKET_LEFT, character: 'ü' }));
+        service.validateKey({ code: Key.QUOTE.code, character: 'ä' });
+        service.validateKey({ code: Key.SEMICOLON.code, character: 'ö' });
+        service.validateKey({ code: Key.BRACKET_LEFT.code, character: 'ü' });
         chai.expect((currentLayout.info as IMacKeyboardLayoutInfo).id).to.equal('com.apple.keylayout.German');
-        service.validateKeyCode(new KeyCode({ key: Key.SEMICOLON, character: 'm' }));
+        service.validateKey({ code: Key.SEMICOLON.code, character: 'm' });
         chai.expect((currentLayout.info as IMacKeyboardLayoutInfo).id).to.equal('com.apple.keylayout.French');
     });
 
@@ -110,7 +110,7 @@ describe('browser keyboard layout provider', function () {
             currentLayout = l;
         });
 
-        service.validateKeyCode(new KeyCode({ key: Key.SEMICOLON, character: 'm' }));
+        service.validateKey({ code: Key.SEMICOLON.code, character: 'm' });
         const spanishLayout = service.allLayoutData.find(data => data.name === 'Spanish' && data.hardware === 'mac')!;
         await service.setLayoutData(spanishLayout);
         chai.expect((currentLayout.info as IMacKeyboardLayoutInfo).id).to.equal('com.apple.keylayout.Spanish');
@@ -121,7 +121,7 @@ describe('browser keyboard layout provider', function () {
     it('restores pressed keys from last session', async () => {
         const { service, container } = setup('mac');
 
-        service.validateKeyCode(new KeyCode({ key: Key.SEMICOLON, character: 'm' }));
+        service.validateKey({ code: Key.SEMICOLON.code, character: 'm' });
         const service2 = container.get(BrowserKeyboardLayoutProvider);
         chai.expect(service2).to.not.equal(service);
         const currentLayout = await service2.getNativeLayout();
@@ -135,7 +135,7 @@ describe('browser keyboard layout provider', function () {
         await service.setLayoutData(spanishLayout);
         const service2 = container.get(BrowserKeyboardLayoutProvider);
         chai.expect(service2).to.not.equal(service);
-        service2.validateKeyCode(new KeyCode({ key: Key.SEMICOLON, character: 'm' }));
+        service2.validateKey({ code: Key.SEMICOLON.code, character: 'm' });
         const currentLayout = await service2.getNativeLayout();
         chai.expect((currentLayout.info as IMacKeyboardLayoutInfo).id).to.equal('com.apple.keylayout.Spanish');
     });

diff --git a/packages/core/src/browser/keyboard/browser-keyboard-layout-provider.ts b/packages/core/src/browser/keyboard/browser-keyboard-layout-provider.ts
@@ -20,14 +20,15 @@ import { isOSX } from '../../common/os';
 import { Emitter } from '../../common/event';
 import { ILogger } from '../../common/logger';
 import { Deferred } from '../../common/promise-util';
-import { NativeKeyboardLayout, KeyboardLayoutProvider, KeyboardLayoutChangeNotifier } from '../../common/keyboard/keyboard-layout-provider';
+import {
+    NativeKeyboardLayout, KeyboardLayoutProvider, KeyboardLayoutChangeNotifier, KeyValidator, KeyValidationInput
+} from '../../common/keyboard/keyboard-layout-provider';
 import { LocalStorageService } from '../storage-service';
-import { KeyCode } from './keys';
 
 export type KeyboardLayoutSource = 'navigator.keyboard' | 'user-choice' | 'pressed-keys';
 
 @injectable()
-export class BrowserKeyboardLayoutProvider implements KeyboardLayoutProvider, KeyboardLayoutChangeNotifier {
+export class BrowserKeyboardLayoutProvider implements KeyboardLayoutProvider, KeyboardLayoutChangeNotifier, KeyValidator {
 
     @inject(ILogger)
     protected readonly logger: ILogger;
@@ -103,22 +104,16 @@ export class BrowserKeyboardLayoutProvider implements KeyboardLayoutProvider, Ke
     }
 
     /**
-     * Test all known keyboard layouts with the given KeyCode. Layouts that match the
-     * combination of key and produced character have their score increased (see class
+     * Test all known keyboard layouts with the given combination of pressed key and
+     * produced character. Matching layouts have their score increased (see class
      * KeyboardTester). If this leads to a change of the top-scoring layout, a layout
      * change event is fired.
      */
-    validateKeyCode(keyCode: KeyCode): void {
-        if (!keyCode.key || !keyCode.character || this.source !== 'pressed-keys') {
+    validateKey(keyCode: KeyValidationInput): void {
+        if (this.source !== 'pressed-keys') {
             return;
         }
-        const accepted = this.tester.updateScores({
-            code: keyCode.key.code,
-            character: keyCode.character,
-            shiftKey: keyCode.shift,
-            ctrlKey: keyCode.ctrl,
-            altKey: keyCode.alt
-        });
+        const accepted = this.tester.updateScores(keyCode);
         if (!accepted) {
             return;
         }
@@ -254,14 +249,6 @@ export interface LayoutProviderState {
     currentLayout?: string;
 }
 
-export interface KeyboardTestInput {
-    code: string;
-    character: string;
-    shiftKey?: boolean;
-    ctrlKey?: boolean;
-    altKey?: boolean;
-}
-
 export interface KeyboardTesterState {
     scores?: { [id: string]: number };
     topScore?: number;
@@ -296,7 +283,7 @@ export class KeyboardTester {
         this.testedInputs.clear();
     }
 
-    updateScores(input: KeyboardTestInput): boolean {
+    updateScores(input: KeyValidationInput): boolean {
         let property: 'value' | 'withShift' | 'withAltGr' | 'withShiftAltGr';
         if (input.shiftKey && input.altKey) {
             property = 'withShiftAltGr';
@@ -329,7 +316,7 @@ export class KeyboardTester {
         return true;
     }
 
-    protected testCandidate(candidate: KeyboardLayoutData, input: KeyboardTestInput,
+    protected testCandidate(candidate: KeyboardLayoutData, input: KeyValidationInput,
         property: 'value' | 'withShift' | 'withAltGr' | 'withShiftAltGr'): number {
         const keyMapping = candidate.raw.mapping[input.code];
         if (keyMapping && keyMapping[property]) {

diff --git a/packages/core/src/browser/keyboard/browser-keyboard-module.ts b/packages/core/src/browser/keyboard/browser-keyboard-module.ts
@@ -16,14 +16,15 @@
 
 import { ContainerModule } from 'inversify';
 import { CommandContribution } from '../../common/command';
-import { KeyboardLayoutProvider, KeyboardLayoutChangeNotifier } from '../../common/keyboard/keyboard-layout-provider';
+import { KeyboardLayoutProvider, KeyboardLayoutChangeNotifier, KeyValidator } from '../../common/keyboard/keyboard-layout-provider';
 import { BrowserKeyboardLayoutProvider } from './browser-keyboard-layout-provider';
 import { BrowserKeyboardFrontendContribution } from './browser-keyboard-frontend-contribution';
 
 export default new ContainerModule((bind, unbind, isBound, rebind) => {
     bind(BrowserKeyboardLayoutProvider).toSelf().inSingletonScope();
     bind(KeyboardLayoutProvider).toService(BrowserKeyboardLayoutProvider);
     bind(KeyboardLayoutChangeNotifier).toService(BrowserKeyboardLayoutProvider);
+    bind(KeyValidator).toService(BrowserKeyboardLayoutProvider);
     bind(BrowserKeyboardFrontendContribution).toSelf().inSingletonScope();
     bind(CommandContribution).toService(BrowserKeyboardFrontendContribution);
 });
diff --git a/packages/core/src/browser/keyboard/keyboard-layout-service.ts b/packages/core/src/browser/keyboard/keyboard-layout-service.ts
@@ -14,10 +14,12 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
-import { injectable, inject } from 'inversify';
+import { injectable, inject, optional } from 'inversify';
 import { IWindowsKeyMapping } from 'native-keymap';
 import { isWindows } from '../../common/os';
-import { NativeKeyboardLayout, KeyboardLayoutProvider, KeyboardLayoutChangeNotifier } from '../../common/keyboard/keyboard-layout-provider';
+import {
+    NativeKeyboardLayout, KeyboardLayoutProvider, KeyboardLayoutChangeNotifier, KeyValidator
+} from '../../common/keyboard/keyboard-layout-provider';
 import { Emitter } from '../../common/event';
 import { KeyCode, Key } from './keys';
 
@@ -43,6 +45,9 @@ export class KeyboardLayoutService {
     @inject(KeyboardLayoutChangeNotifier)
     protected readonly layoutChangeNotifier: KeyboardLayoutChangeNotifier;
 
+    @inject(KeyValidator) @optional()
+    protected readonly keyValidator?: KeyValidator;
+
     private currentLayout?: KeyboardLayout;
 
     protected updateLayout(newLayout: NativeKeyboardLayout): KeyboardLayout {
@@ -103,13 +108,17 @@ export class KeyboardLayoutService {
 
     /**
      * Called when a KeyboardEvent is processed by the KeybindingRegistry.
-     * If the keyboard layout provider supports KeyCode validation, delegate to it.
+     * The KeyValidator may trigger a keyboard layout change.
      */
     validateKeyCode(keyCode: KeyCode): void {
-        // tslint:disable-next-line:no-any
-        const provider = this.layoutProvider as any;
-        if (typeof provider.validateKeyCode === 'function') {
-            provider.validateKeyCode(keyCode);
+        if (this.keyValidator && keyCode.key && keyCode.character) {
+            this.keyValidator.validateKey({
+                code: keyCode.key.code,
+                character: keyCode.character,
+                shiftKey: keyCode.shift,
+                ctrlKey: keyCode.ctrl,
+                altKey: keyCode.alt
+            });
         }
     }
 

diff --git a/packages/core/src/common/keyboard/keyboard-layout-provider.ts b/packages/core/src/common/keyboard/keyboard-layout-provider.ts
@@ -31,6 +31,20 @@ export interface KeyboardLayoutChangeNotifier {
     onDidChangeNativeLayout: Event<NativeKeyboardLayout>;
 }
 
+export interface KeyValidationInput {
+    code: string;
+    character: string;
+    shiftKey?: boolean;
+    ctrlKey?: boolean;
+    altKey?: boolean;
+}
+
+export const KeyValidator = Symbol('KeyValidator');
+
+export interface KeyValidator {
+    validateKey(input: KeyValidationInput): void;
+}
+
 export interface NativeKeyboardLayout {
     info: IKeyboardLayoutInfo;
     mapping: IKeyboardMapping;