eclipse-theia / theia

Eclipse Theia is a cloud & desktop IDE framework implemented in TypeScript.
http://theia-ide.org
Eclipse Public License 2.0
19.87k stars 2.48k forks source link

WebviewPanelSerializer not working in Theia #12834

Closed jcortell68 closed 1 year ago

jcortell68 commented 1 year ago

Description

A VS Code extension can register a WebviewPanelSerializer . Such an object is exercised by VS Code to recreate a webview panel's content in the event that the previous VS Code instance was terminated with the webview open. It appears Theia supports this (the associated API is there), but the serializer isn't exercised.

I tried the same thing in VS Code and it works as expected.

Part of this mechanism involves declaring an activation event so that if Theia needs to recreate a webview of type, e.g., myextWebviewPanelType, then your extension will be activated on VS Code launch, such that it can register the serializer (presumably before Theia needs to make use of it).

In package.json, I have

"activationEvents": ["onWebviewPanel:myextWebviewPanelType"],

and in fact, my extension is being activated on launch because of it. However, the serializer's deserializeWebviewPanel method is never called.

Steps to Reproduce:

git clone -b webview_panel_bug2 https://github.com/jcortell68/theiasandbox
cd theiasandbox
yarn
yarn start:electron
  1. Open the webview via the command Open Webview, which the extension contributes. You should see the webview.
  2. Close the Theia app
  3. Re-launch the Theia app
  4. Note that the My Webview Panel is open, but has no content. Also, looking at stdout, you can see that console.log() in the serializer code doesn't appear. I.e., it was not called.

Additional Information

vatsal-uppal-1997 commented 1 year ago

Hi @jcortell68,

I was debugging this issue and the following seems to be the root cause:

The flow for restoring a web view seems to be as follows (see hosted-plugin.ts)

  1. In the class HostedPluginSupport there is a method called doLoad
  2. The doLoad method is responsible for calling the restoreWebviews method at the end of it
  3. The restoreWebviews method further attempts to restore each web view by calling the restoreWebview method
  4. But at the time of calling the restoreWebview method, no webviewRevivers are registered yet
  5. After the call to restoreWebview, the registerWebviewReviver method is called via the rpc protocol to register a webviewReviver

So as you can see webviewRevivers are being registered after we have attempted to restore the view.

Purposed solution:

Instead of trying to restore the web views in the doLoad method, we should instead try to restore a web view when its webviewReviver has been registered and for that I've done the following changes

Revised flow:

  1. In the preserveWebview method which is called whenever we're restoring a webview, we'll set a map webviewsToRestore with the key being webview.viewType and the value being the webview itself. We'll also emit an activation event to activate the said web view.
  2. After the activation we would subsequently receive a call to the registerWebviewReviver method to register a webviewReviver and at that time we'll check if the previously set webviewsToRestore map has an entry for the given viewType and if it does we'll then call the restoreWebview method but this time it should work correctly as it would correctly find the appropriate webviewReviver

The way I am testing this is quite janky since I am modifying the package.json in your test repo by adding an entry like:

"@theia/plugin-ext": "C:\\Users\\vatsal\\theia\\packages\\plugin-ext"

If there's a better way to test please do tell

jfaltermeier commented 1 year ago

@vatsal-uppal-1997 I've tested your changes and they fix the issue for me as well. Changes look good to me as well. When it comes to testing, you could build a vsix (https://code.visualstudio.com/api/working-with-extensions/publishing-extension#usage vsce package without publish) from the test repo and simply install it in theia's example browser or electron app (https://github.com/eclipse-theia/theia/blob/master/doc/Developing.md#quick-start) run from source. This way all Theia sources are in sync. I'll assign the issue to you, if you don't mind

jcortell68 commented 1 year ago

@vatsal-uppal-1997 Thanks for the investigation and fix!

vatsal-uppal-1997 commented 1 year ago

Hi @jfaltermeier,

Thanks for testing my changes and guiding me regarding how to test this issue 😄 !

Consequently, I've raised this PR to fix this issue.

Hope this helps!