Fix window maximization when using splash screen

[tortmayr]

What it does

Avoid eager restore of maximized state in TheiaElectronWindow.restoreMaximizeState
Calling window.maximize() implicitly also makes the window visible.
Therefore we have to check if the window is already visibible before restoring t he maximizeState.
If the window is not yet visible yet we restore the state once the window is shown.

This fixes and issue were both the splashscreen and the preload window are be visible at the same time.

Fixes #14218

How to test

1. Build electron example app
2. Start electron example app
3. Maximize window and close app
4. Start app again

Only the splash screen should be shown, main window should remain invisible until the splash screen is hidden.

Follow-ups

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

[sdirix]

I'm not sure that here is the best place to put the maximize logic. I agree that this fixes the issue, however this depends on the fact that we know that the main application will call show once the application state is 'ready'.

What do you think of the following:

• If a Window is immediately shown (i.e. this.options.show === true || typeof this.options.show === 'undefined'), we directly call the restoreMaximizedState
• If a Window is not immediately shown we
  • return a Proxy of BrowserWindow
  • The proxy overrides the show of BrowserWindow
  • On the first time show is called, we also implicitly call restoreMaximizedState

[tortmayr]

IMO using a proxy approach here is kind of an overkill. Maybe it would be better to move the maximization logic into the main application entirely. Here we know exactly when the main window will be shown and can invoke the maximization at this point if nencessary.
It could look like this:

diff
diff --git a/packages/core/src/electron-main/electron-main-application.ts b/packages/core/src/electron-main/electron-main-application.ts
index b464c916a84..13f7fe127b7 100644
--- a/packages/core/src/electron-main/electron-main-application.ts
+++ b/packages/core/src/electron-main/electron-main-application.ts
@@ -334,19 +334,22 @@ export class ElectronMainApplication {
                 });
                 if (this.isShowSplashScreen()) {
                     console.log('Showing splash screen');
-                    this.configureAndShowSplashScreen(this.initialWindow);
+                    this.configureAndShowSplashScreen(this.initialWindow, options.isMaximized);
                 }

                 // Show main window early if windows shall be shown early and splash screen is not configured
                 if (this.isShowWindowEarly() && !this.isShowSplashScreen()) {
                     console.log('Showing main window early');
                     this.initialWindow.show();
+                    if (options.isMaximized) {
+                        this.initialWindow.maximize();
+                    }
                 }
             });
         }
     }

-    protected async configureAndShowSplashScreen(mainWindow: BrowserWindow): Promise<BrowserWindow> {
+    protected async configureAndShowSplashScreen(mainWindow: BrowserWindow, isMaximized?: boolean): Promise<BrowserWindow> {
         const splashScreenOptions = this.getSplashScreenOptions()!;
         console.debug('SplashScreen options', splashScreenOptions);

@@ -382,6 +385,9 @@ export class ElectronMainApplication {
             cancelTokenSource.cancel();
             if (!mainWindow.isVisible()) {
                 mainWindow.show();
+                if (isMaximized) {
+                    mainWindow.maximize();
+                }
             }
             splashScreenWindow.close();
         };
diff --git a/packages/core/src/electron-main/theia-electron-window.ts b/packages/core/src/electron-main/theia-electron-window.ts
index 720865f8f59..4faa5852fb7 100644
--- a/packages/core/src/electron-main/theia-electron-window.ts
+++ b/packages/core/src/electron-main/theia-electron-window.ts
@@ -86,7 +86,6 @@ export class TheiaElectronWindow {
         if (!this.options.preventAutomaticShow) {
             this.attachReadyToShow();
         }
-        this.restoreMaximizedState();
         this.attachCloseListeners();
         this.trackApplicationState();
         this.attachReloadListener();
@@ -185,14 +184,6 @@ export class TheiaElectronWindow {
         return TheiaRendererAPI.requestClose(this.window.webContents, reason);
     }

-    protected restoreMaximizedState(): void {
-        if (this.options.isMaximized) {
-            this._window.maximize();
-        } else {
-            this._window.unmaximize();
-        }
-    }
-
     protected trackApplicationState(): void {
         this.toDispose.push(TheiaRendererAPI.onApplicationStateChanged(this.window.webContents, state => {
             this.applicationState = state;

WDYT?

Sidenote: options.show is currently set to false by default in Theia and not considered at all in the initialization logic.
I think we should keep it that way.

[sdirix]

Fine by me in general. We just need to make sure that when the window is shown automatically, without the main application calling show itself, we also restore the maximized state.

[tsmaeder]

@sdirix @tortmayr what's the state on this one?

[sdirix]

@sdirix @tortmayr what's the state on this one?

@tortmayr Can you update the PR as discussed?

[tortmayr]

Sorry, for the long roundtrip. I was busy with other projects and didn't find the time to work on this PR.
I have now pushed an update.
In the end I found a pretty simple solution for this issue.
In the restoreMaximizeState method we simply have to check if the window is already visible.
If so restore the state directly.
If not we have listen for the next show event on the window and restore the state then.

@sdirix Could you please rereview this change?

[sdirix]

Works for me! Great final solution.