Fix RPC proxy handler notifications and requests order

[rschnekenbu]

Draft PR to discuss TestController initialization issues when dealing with extension

How to test:

• install test-extension-sample (a dummy mathematical expression tester in markdown files), from vscode extension samples:
  • src: test-provider-sample-0.0.1-src.zip
  • zip vsix: test-provider-sample-0.0.1.zip

On theia master, the extension does not activate nicely. Error messages are displayed in the console, where I get some No test controller with id mathTestController found. Looking at the code, the extension registers at the same time the test controller, the test profile and some other things during activation. We get some race condition here between initialization of the TestController on main and its usage when the profile is registered. When profile is registered, testing main uses the withController, where it is not yet available. So the error messages.

I pushed a dirty fix, but I would like to discuss with you the best way to handle this issue. Timeout is probably not the best solution, or trying x times is not the best. What would you think as a good implementation here? Knowing of course that we cannot change the way the extension does its activation.

The fix on TestItem is also a test for some tree flickering I get on the Test Items => the extension keeps sending some updates on test items, and as far as I understand the framework, the notifyPropertyChange implementation does not take into account the case where the value is similar to the previous one, causing many tree refreshes. But this part can be ignored for now ;)

[msujew]

@rschnekenbu The way I usually do this is via events. See for example:

https://github.com/eclipse-theia/theia/blob/c37254f2cfc22cd081c76ae505eeb793bca9b47f/packages/plugin-ext/src/plugin/notebook/notebooks.ts#L315-L333

It doesn't require a timeout (aside from the one that rejects the promise) and should be usable in here as well. You probably just need to introduce a new event emitter that can be used here.

[tsmaeder]

@msujew thanks for the input, but I think the problem is much more serious: in proxy-handler.ts, we have this code:

    protected handleRequest(method: string, args: any[]): Promise<any> {
        return this.rpcDeferred.promise.then(() => this.target[method](...args));
    }

    protected onNotification(method: string, args: any[]): void {
        this.target[method](...args);
    }
}

Handling of requests is async, while sending notification is synchronous. This can lead to notifications being handled before a request that that has been sent before the notification. Hence we get a notification for a test controller which has not been created yet via a request.

[msujew]

@tsmaeder Oh I see. Indeed, that is more severe...

[rschnekenbu]

It seems that we can have a quick fix for this one (sending notifications also on Promise.then()...), but this may lead to the same issues we had in the test area. Things were working but not always correct. We may have to check if all extension APIs still work with this change. Thanks a lot to both of you for the support here!

[rschnekenbu]

With fixes on proxy handler and some other order issues on notification for Test Controller, the sample extension should be running fine.