Unable to restore Theia state when webview and several editor tabs are opened

[vitaliy-guliy]

What it does

Fixes an error with restoring the editor area when a webview and several editors are opened.

Short video demonstrating a problem is here https://www.youtube.com/watch?v=9ir1hTov350

How to test

Follow 'Steps to reproduce' in the original issue #8620

This PR is also fixes #6877. I haven't investigated sources of GitLens extension, but the bug with webview+preferences is not reproducible anymore.

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[tsmaeder]

I'm not convinced this is the right fix: if we don't have a WebviewPanelSerializer, shouldn't we just recreate the webview to it's default state? After all, there are webviews that simply do not have mutable state.
Could you point me to the code that disposes the widget if there's now WebviewPanelSerializer? I didn't find it.

bla

[vitaliy-guliy]

I'm not convinced this is the right fix: if we don't have a WebviewPanelSerializer, shouldn't we just recreate the webview to it's default state? After all, there are webviews that simply do not have mutable state.

Theia finds a WebviewPanelSerializer when saving the layout, as the serializer is used to get the webview html content.

If the serializer is not registered for the webview, theia saves only widget ID, factory ID and empty content.

theia/packages/plugin-ext/src/hosted/browser/hosted-plugin.ts

Lines 204 to 209 in 8e3da24

	widget.storeState = () => {
	if (this.webviewRevivers.has(widget.viewType)) {
	return storeState();
	}
	return {};
	};

Then when restoring the state after reload, Theia

• creates a widget by a widget factory (which is 'plugin-webview')
  

  theia/packages/core/src/browser/shell/shell-layout-restorer.ts

  Line 317 in 8e3da24

  const widget = await this.widgetManager.getOrCreateWidget(desc.constructionOptions.factoryId, desc.constructionOptions.options);

• tries to restore the widget, but cannot. oldState object is empty, just because empty value was saved. So, it switched to widget.dispose()
  

  theia/packages/plugin-ext/src/hosted/browser/hosted-plugin.ts

  Lines 210 to 217 in 8e3da24

  	widget.restoreState = oldState => {
  	if (oldState.viewType) {
  	restoreState(oldState);
  	this.preserveWebview(widget);
  	} else {
  	widget.dispose();
  	}
  	};

• on the last check undefined value is returned for disposed widget
  

  theia/packages/core/src/browser/shell/shell-layout-restorer.ts

  Lines 336 to 338 in 8e3da24

  	if (widget.isDisposed) {
  	return undefined;
  	}

[tsmaeder]

The if (oldState.viewType) check could win an obfuscated code contest ;-). This could use a commment.

[vitaliy-guliy]

Could somebody check this PR?

[vitaliy-guliy]

@akosyakov @vince-fugnitto could you please take a look?

[tsmaeder]

testing....

[vince-fugnitto]

Sorry I have not viewed the pull-request sooner, I was busy with EclipseCon.
I tested the functionality only at the moment, I see that editors are properly restored with this patch as compared to master.

[tsmaeder]

Hmh...the pressing F5 does not restore the order of the editors: python files always first. Checking master...

[tsmaeder]

Master seems to respect editor order. Need to look at that code again.

[tsmaeder]

Here's what I do:

1. start the browser example
2. Open a python file, two json files and another python file
3. Observe: the editor tabs are in the order they are opened from left to right
4. Close the "hello" welcome page
5. Press F5
6. Observe: the two python files are the first editor tabs from the beginning.
7. Observe: the "hello" welcome page is then opened as the second editor tab from the left.

[vitaliy-guliy]

Here's what I do:

1. start the browser example
2. Open a python file, two json files and another python file
3. Observe: the editor tabs are in the order they are opened from left to right
4. Close the "hello" welcome page
5. Press F5
6. Observe: the two python files are the first editor tabs from the beginning.
7. Observe: the "hello" welcome page is then opened as the second editor tab from the left.

What devfile did you use to test?

After refreshing the page, the order of opened editors must be restored.
But if the webview is not persisted, it will be opened again the next to active editor tab.

[tsmaeder]

What devfile did you use to test?

No devfile, plain Theia.

After refreshing the page, the order of opened editors must be restored.
But if the webview is not persisted, it will be opened again the next to active editor tab.

That is correct, but the order of editors is not preserved in my case. Maybe we're iterating over property keys instead of an array somewhere?

[vitaliy-guliy]

@tsmaeder I have just updated and double-checked the PR. The order of opened editors is preserved after refreshing the page.

[tsmaeder]

🤦 I'll have to double-check, then. I'm pretty confident I did not dream it.

[tsmaeder]

I'm still seeing reordering of the open files running the browser example when opening the following directory:

First.zip

1. Open hello.py, then launch.json, then settings.json, then config.py
2. Refresh the browser
3. Files are in order hello.py config.py launch.json settings.json

[vitaliy-guliy]

I reproduced the bug with your project. Sometimes (not always) the ordering of files is broken.
It could be because the speed of restoring of editors could be different for different type of files.
In my PR I changed a bit the order of inserting the widgets to the array. Then, the order could be broken if time for opening of the first widget is bigger comparing to the second one. In this case the second file will appear the first.

[tsmaeder]

@vitaliy-guliy of course:

const widget = await this.convertToWidget(descs[i], context);
if (widget) {
  widgets.unshift(widget);
}

compiles to

this.convertToWidget(descs[i], context).then(()=> {
  if (widget) {
    widgets.unshift(widget);
  }
);

[tsmaeder]

I'm glad I'm not crazy ;-)

[vitaliy-guliy]

I created a PR #8680 with another approach. In that PR Theia does not store the webview if the serializer is not set.

[azatsarynnyy]

I've tested it with the provided plug-in. The previously opened files are restored well. But with restoring a WebView I see two problems:

• the position of the WebView tab is not the same as it was before reloading the entire page
• the contributed WebView is restored anyway, even if I've already closed it before reloading the Theia page 😄

[vitaliy-guliy]

I've tested it with the provided plug-in. The previously opened files are restored well. But with restoring a WebView I see two problems:

• the position of the WebView tab is not the same as it was before reloading the entire page

It's because the webview is not persisted and the plugin opens it next to active tab.

• the contributed WebView is restored anyway, even if I've already closed it before reloading the Theia page

The plugin opens a webview always when Theia starts.

[ericwill]

I created a PR #8680 with another approach. In that PR Theia does not store the webview if the serializer is not set.

Does that mean this PR should be closed?