Frameless backward compatibility issues

[donotcodeit]

Minimum reproduceable app: https://github.com/donotcodeit/ChromelyMin51/tree/repro/frameless-backward-compatibility-issues

There's list of issues, which wasn't present or was fixed on v5.0:

• Window now has thin frame. Frameless app should not have a frame because of frameless nature.
• Changing to fullscreen, than restore, causes appearing of a black edge.
• Fullscreen window is not actually fullscreen anymore. It become stretched to the screen size, but we still able to drag'n'drop and resize it.

It is probably related to changed Window classes (WinApi related). I've tried to change them back as in v5.0, but app will crash on changing to fullscreen mode (related to #264).

I will try to fix all abovementioned backward compatibility issues, but it will take a time, because code base was changed a lot.

[mattkol]

@donotcodeit

Please note:

• Windows Only - Borderless will use Rossy's borderless that will not require Window Subclassing. Depending on feedbacks the current Window Subclassing may be re-added as an option.

https://github.com/chromelyapps/Chromely/issues/225

• https://github.com/chromelyapps/Chromely/blob/960f8ac1c97c9fd73591bd2f0828d4cee5bf5900/src/Chromely/NativeHosts/WinHost/ChromelyWinFramelessHost.cs#L30

Personally, I have not been able to make Fullscreen and Kiosk mode work with DWM approach, so if we need to bring back the subclass approach, we could, but I'd rather we try fix the DWM approach first.

• Also what Windows version are you using to test?

Thanks.

[xplicit]

I found that 1px frame on frameless window is created by this part of code:

https://github.com/chromelyapps/Chromely/blob/960f8ac1c97c9fd73591bd2f0828d4cee5bf5900/src/Chromely/NativeHosts/WinHost/ChromelyWinFramelessHost.cs#L88-L96

Unfortunately, disable marging also disables ability to resize the window. As far as I remember ability to resize frameless window was one of the major issue which we fought on previous version of chromely and it was resolved by using HIT_TEST on borders.

[donotcodeit]

@mattkol I'm using Win10 x64 to test.

Regarding Rossy's borderless: when is it planned, do you have approximate date? Also as I see it is C lib, that will be somehow integrated to .NET code. I hope integration will still have ability to override at least wndproc.

[donotcodeit]

@mattkol do we got it correct: current DWM approach is a Rossy's solution rewritten to C#?

[mattkol]

@xplicit

Unfortunately, disable marging also disables ability to resize the window. As far as I remember ability to resize frameless window was one of the major issue which we fought on previous version of chromely and it was resolved by using HIT_TEST on borders.

This seems like the hardest part of the DWM approach. What was there was more of a hack, but works for now. I was reluctant to use subclass to get the HIT_TEST, but we may resort to that.

[mattkol]

@donotcodeit

Regarding Rossy's borderless: when is it planned, do you have approximate date?

July 22, 2020 it was announced. It was a tagged issue for a month or so..
First pre-release - https://www.nuget.org/packages/Chromely/5.1.83-pre01

Also as I see it is C lib, that will be somehow integrated to .NET code. I hope integration will still have ability to override at least wndproc.

Yes. https://github.com/chromelyapps/Chromely/blob/960f8ac1c97c9fd73591bd2f0828d4cee5bf5900/src/Chromely/NativeHosts/WinHost/ChromelyWinFramelessHost.cs#L105

do we got it correct: current DWM approach is a Rossy's solution rewritten to C#?

Yes. Like I said earlier and in #225, we could revert back to the subclass approach. Rossy borderless could just stay as an option to launch non-resizable borderless window that is not a pop-up.

[donotcodeit]

Like I said earlier and in #225, we could revert back to the subclass approach.

@mattkol it would be great and will fix all backward compatibility issue at once. Is it simple and doesn't take a long time?

[mattkol]

it would be great and will fix all backward compatibility issue at once. Is it simple and doesn't take a long time?

It is exactly what we have in 5.0. Putting back the code is not a difficult thing, it is making sure everything works that takes time, if we are all hands on deck now, we can start the migration ASAP.

My suggestion is this, we have 2 approaches and see which one we keep.

1. We apply subclass to the DWM approach to get HIT_TEST as @xplicit pointed out.
2. We bring back the old subclass.

[xplicit]

Yes. Like I said earlier and in #225, we could revert back to the subclass approach. Rossy borderless could just stay as an option to launch non-resizable borderless window that is not a pop-up.

Yeah, it would be great if we get previous functionality working (resizing frameless window, no borders on frameless window, ability to drag window by using -webkit-app-region: drag, ability to maximize and restore window);

[xplicit]

My suggestion is this, we have 2 approaches and see which one we keep.

1. We apply subclass to the DWM approach to get HIT_TEST as @xplicit pointed out.
2. We bring back the old subclass.

We can test operatively both approaches to see which is working.

[mattkol]

1. We apply subclass to the DWM approach to get HIT_TEST as @xplicit pointed out.
2. We bring back the old subclass.

Both options are working.

But the DWM approach is missing top resizing. https://github.com/chromelyapps/Chromely/blob/78b1cd0beecce40b9f1e2308365a808253017cae/src/Chromely/NativeHosts/WinHost/Frameless/DwmFramelessController.cs#L208

With it, the titlebar comes back. Does not matter what the DwmResizeGrip is:

nonclient.top += _framelessOption.DwmResizeGrip;

Without it (DwmResizeGrip = 0) you get:

// nonclient.top += _framelessOption.DwmResizeGrip;

[xplicit]

@mattkol could please describe what was changed in the latest commit, what should we test? I tried to run frameless window with this commit and still see thin border on left/bottom/right edge (on top it dissapeared but I can't resize top anymore like I did in prev version)

[xplicit]

If I change DwmResizeGrip = 0, then I loose ability to resize on all edges (but in this case thin white frame goes away as we expect)

[mattkol]

@xplicit forgot to add how to use.

Using subclass

   class Program
    {
        [STAThread]
        static void Main(string[] args)
        {
            var config = DefaultConfiguration.CreateForRuntimePlatform();
            config.WindowOptions.WindowFrameless = true;

            AppBuilder
            .Create()
            .UseConfig<DefaultConfiguration>(config)
            .UseApp<BasicApp>()
            .Build()
            .Run(args);
        }
    }

    public class BasicApp : ChromelyBasicApp
    {
        public override void ConfigureServices(IServiceCollection services)
        {
        }
    }

Using DWM:

    class Program
    {
        [STAThread]
        static void Main(string[] args)
        {

            AppBuilder
            .Create()
            .UseApp<DwmApp>()
            .Build()
            .Run(args);
        }
    }

    public class DwmApp : ChromelyFramelessApp
    {
        public override void ConfigureServices(IServiceCollection services)
        {
        }
    }

[mattkol]

@xplicit

If I change DwmResizeGrip = 0, then I loose ability to resize on all edges (but in this case thin white frame goes away as we expect)

This was trying to fix the issue raised here:

I found that 1px frame on frameless window is created by this part of code:

Did the first approach show the the what edge?

At this stage I am tempted to stop the DWM approach. Resizing seems to be an issue. But we can conclude on this.

[xplicit]

Looks like switching to ChromelyBasicApp from ChromelyFramelessApp resolves our issue with thin white border. App still has a border but with ChromelyBasicApp it's black and it is what we want.

However I did not fully test the application yet because I have an issue with registering services. In Build() method we have such sequence:

        _chromelyApp.ConfigureServices(_serviceCollection);

        // This must be done before registring core services
        RegisterUseComponents(_serviceCollection);

        _chromelyApp.ConfigureCoreServices(_serviceCollection);
        _chromelyApp.ConfigureServiceResolvers(_serviceCollection);
        _chromelyApp.ConfigureDefaultHandlers(_serviceCollection);
        _serviceProvider = _serviceCollection.BuildServiceProvider();
        _chromelyApp.Initialize(_serviceProvider);
        _chromelyApp.RegisterControllerRoutes(_serviceProvider);

ConfigureServices() can be overridden while other methods are sealed and can't be overridden. We need to register service which depends on IChromelyConfiguration . IChromelyConfiguration is added in ConfigureCoreServices() so we need to override some method which takes IServiceCollection as param and follows after ConfigureCoreServices() or override it. But we can't because they are sealed.

So I think this can be two solutions: or remove sealed from ConfigureCoreServices() and ConfigureDefaultHandlers() or place ConfigureServices() after ConfigureCoreServices() or add some virtual methods for users if these methods must not be overridden in any case.

[xplicit]

Nevermind, I was able to restructure our solution and move code which is depended on IChromelyConfiguration to Initialize()

[mattkol]

@xplicit

App still has a border but with ChromelyBasicApp it's black and it is what we want.

It is frameless with a border? Is it possible you share this border.

On ConfigureCoreServices there was a chat on this here - https://github.com/chromelyapps/Chromely/pull/249#discussion_r500965102

I will remove the DMW code. The objective originally was to avoid using subclass, but the resize issue makes it an unviable option.

[donotcodeit]

@mattkol I've updated demo to show the black border with subclass approach: https://github.com/donotcodeit/ChromelyMin51/tree/repro/frameless-backward-compatibility-issues

But it is OK for us for now.

[mattkol]

Please note that the DWM approach has been removed - https://github.com/chromelyapps/Chromely/commit/3996f9d7eb0b63a6281dae83bc1071f699fa4888.

[mattkol]

@donotcodeit @xplicit

I see the black border. Looks like that is normal. When activated/highlighted it shows. There is likely a way to fix it, but not sure what real benefit fixing it brings.

[mattkol]

@donotcodeit

I see that you created a custom host class and registered it - https://github.com/donotcodeit/ChromelyMin51/blob/59ddc4a4ddc1871cfddfb4fa4889a7479ac1b37a/src/ChromelyMin51/Program.cs#L45

Not sure it is necessary, unless this is in preparation for future work.

[mattkol]

@xplicit @donotcodeit

Please let me know if you notice anything else, we could have new nugets by weekend. Thanks.

[xplicit]

@mattkol we are testing our solution now, on x64 all works fine, but we have a crash for 5.1 on x86 platform, we're trying to localize the issue now to see what was the cause. It might be not a chromely issue, but for version 5.0 we didn't meet any crashes on x86. I will post when get more info.

[xplicit]

just FYI: bisecting in our solution brings me that crash on x86 occurs on commit when the switched from ChromelyFramelessApp to ChromelyBasicApp.

[xplicit]

More info after debugging:

1. This crash occurs only on x86 platform

2. Crash occurs immediately after start, window is created, but web content is not rendered.

3. If I comment this line then no crash, I can see window with web content, all is working, but of course no dragging. https://github.com/chromelyapps/Chromely/blob/master/src/Chromely/NativeHosts/WinHost/Frameless/DefaultWindowMessageInterceptor.cs#L120

4. If I add here https://github.com/chromelyapps/Chromely/blob/master/src/Chromely/NativeHosts/WinHost/Frameless/DefaultWindowMessageInterceptor.cs#L113

   if (!isHost) return;

   crash does not occur. I can see window with web content, but when I try to drag window it crashes. The last operation is calling WindowProc here https://github.com/chromelyapps/Chromely/blob/master/src/Chromely/NativeHosts/WinHost/Frameless/DefaultWindowMessageInterceptor.cs#L194

5. (Unrelated to this issue) The variable wndProcPtr can be garbage collected and memory of delegate is replaced with other data. Need to change it to class member and prevent it from garbage collection.

https://github.com/chromelyapps/Chromely/blob/master/src/Chromely/NativeHosts/WinHost/Frameless/DefaultWindowMessageInterceptor.cs#L119

[xplicit]

I tried to replace SetWindowLong by SetWindowSubclass but this didn't work, DragWindowHandler.WndProc was not called. Then I noticed that DragWindowHanlder is executed on the worker thread not main thread. These windows function should be called from the main UI thread.I tried to hack and saw if setting thread to main helps and I mark the thread with STA by using Thread.CurrentThread.SetAppartmentState(AppartmentState.STA). After doing this I get calls to DragWindowHanlder.WndProc. I think calling DragWindowHandler from main thread with using SetWindowSubclass instead of SetWindowLong or change number of params in WndProc might help to resolve the crash.

[mattkol]

@xplicit

I tried to replace SetWindowLong by SetWindowSubclass but this didn't work, DragWindowHandler.WndProc was not called ..

Reason I just wish we do not have to use subclass. Real headache. I think it was due to use of non-pointer for SetWindowLong.

Please try it again.

[donotcodeit]

@mattkol last fix resolved the problem. Thanks!

[xplicit]

@mattkol thanks, we will test new build today and I will write results

[xplicit]

We tested on Win7, Win8.1 and Win10. Both x86/x64 versions, no issues were found so I think this issue can be closed.

[mattkol]

@xplicit @donotcodeit great efforts! Thanks.