Recollect Java bundles when granting workspace trust

[testforstephen]

Signed-off-by: Jinbo Wang jinbwan@microsoft.com

Fixes #1994

In untrusted workspaces, those extensions that do not declare support for workspace trust are not visible to the extensions.all API. Since the variable clientOptions is preconfigured when activating the Java extension, it becomes outdated after we trust the workspace. The list of Java bundles needs to be recollected before switching to standard mode.

It seems that extensions.all still does not include the new available Java extensions during the callback onDidGrantWorkspaceTrust. This looks like a VS code bug. A workaround would be to add a timeout to allow Java extensions to get the latest available Java bundles.

[testforstephen]

// @Eskibear

[rgrunber]

If there's some way to use extensions.onDidChange() and something like https://github.com/redhat-developer/vscode-microprofile/blob/master/src/languageServer/plugin.ts#L45-L68 to avoid the timeout that would be great. Otherwise I'd just confirm that the timeout can be made no smaller.

[testforstephen]

Update: extensions.onDidChange() is not called when granting workspace trust.

We also created an issue microsoft/vscode#127055 to ask extension.all API to returns latest result when onDidGrantWorkspaceTrust. Before that's addressed, timeout could be a temporary workaround for us now.

Update 2:
The extensions.onDidChange() API works in stable VS Code. Previously what i try is VS Code Insider version, which has an issue microsoft/vscode#127067 that cause this API is not called during granting workspace trust.

So if we choose to use the solution of extensions.onDidChange(), what should we do when responding to onDidGrantWorkspaceTrust API? Start the standard server as usual, then once some new Java bundles are detected, kill the old LS silently and start a new LS?

But I also see some side effects if we choose to restart LS silently with extensions.onDidChange(). For example, I am debugging a Java application. Then I want to install the java-test-runner extension to try unit testing. extensions.onDidChange() will detect the installation of a new Java extension and silently restart LS. this will silently kill my current Java debug session. I'm not sure that automatically closing my current work without my knowledge is a good behavior.

[rgrunber]

If the extensions.onDidChange() route produces unwanted behaviour in some cases, I think both @fbricon and I agreed that the current solution of waiting 1s could work with a change :

• Poll every ~100ms (timeout after 1s) and check whether contributing javaExtensions have changed (eg. https://github.com/redhat-developer/vscode-java/blob/master/src/plugin.ts#L60 ).

This way at least we never wait the maximum amount of time. I know we've discussed that we can't tell whether the change came from the workspace trust action, but how likely is it for a user to be able to perform an installation action near the "trust action" and have this still fail ?

[testforstephen]

I have changed to polling every 100ms and timeout after 1s. I tried it in two machines, both them could get the extension changes at 400ms ~500ms.
Machine 1 - Windows 10
Intel(R) Xeon(R) CPU E5-1620 v4 @ 3.50GHz 3.50 GHz
32.0 GB

Machine 2 - macOS
MacBook Pro (13-inch, 2016, Four Thunderbolt 3 Ports)
Processor: 2.9 GHz Dual-Core Intel Core i5
Memory: 8 GB 2133 MHz LPDDR3

how likely is it for a user to be able to perform an installation action near the "trust action" and have this still fail?

Once the standard server is initialized, we have extra logic to cover the case of installing new extension.

vscode-java/src/standardLanguageClient.ts

Lines 402 to 406 in 7ac2be5

	if (extensions.onDidChange) {// Theia doesn't support this API yet
	extensions.onDidChange(() => {
	onExtensionChange(extensions.all);
	});
	}

If you're talking about the time window between the action of starting standard server and registering extensions.onDidChange callback, yes, it is possible to miss new extensions if they are coincidentally installed at that time. This should also exist in current stable version 0.79.2, we might need a new PR to systematically solve that case.

[rgrunber]

Works for me.