cefsharp / CefSharp

.NET (WPF and Windows Forms) bindings for the Chromium Embedded Framework
http://cefsharp.github.io/
Other
9.87k stars 2.92k forks source link

WPF - not interactive/rendering after removing and readding to window until its next resize #3961

Closed igandrews closed 2 years ago

igandrews commented 2 years ago

Bug Report

Need a window which will host 2 ChromiumWebBrowsers:

<Window x:Class="CefSharp.Wpf.Example.BugWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        xmlns:local="clr-namespace:CefSharp.Wpf.Example"
        mc:Ignorable="d"
        Title="BugWindow" Height="800" Width="800">
    <Grid>
        <Grid.RowDefinitions>
            <RowDefinition Height="Auto" />
            <RowDefinition Height="*" />
        </Grid.RowDefinitions>
        <Grid.ColumnDefinitions>
            <ColumnDefinition />
            <ColumnDefinition />
        </Grid.ColumnDefinitions>
        <StackPanel Orientation="Horizontal" Grid.ColumnSpan="2">
            <Button Content="Add" Click="OnAddBrowser" />
            <Button Content="Remove" Click="OnRemoveBrowser" />
        </StackPanel>
        <Border Grid.Row="1" Grid.Column="0" x:Name="StaticBrowser" />
        <Border Grid.Row="1" x:Name="BrowserSite" Grid.Column="1" />
    </Grid>
</Window>
using System.Windows;

namespace CefSharp.Wpf.Example
{
    public partial class BugWindow : Window
    {
        private CefSharp.Wpf.ChromiumWebBrowser browser;

        public BugWindow()
        {
            InitializeComponent();

            this.StaticBrowser.Child = new CefSharp.Wpf.ChromiumWebBrowser("http://www.msn.net");
        }

        private void OnAddBrowser(object sender, RoutedEventArgs e)
        {
            if (browser == null)
            {
                browser = new CefSharp.Wpf.ChromiumWebBrowser("http://www.msn.net");
            }

            this.BrowserSite.Child = browser;
        }

        private void OnRemoveBrowser(object sender, RoutedEventArgs e)
        {
            if (browser != null)
            {
                this.BrowserSite.Child = null;
            }
        }
    }
}

Steps:

  1. Run the app. [The left hand side of the window will have a chromiumwebbrowser created and initialized in the ctor.]
  2. Click the Add button. [The right hand side of the window will get a cwb. Notice how you can scroll the web page, etc.]
  3. Click the Remove button. [The right hand side's browser is empty]
  4. Click the Add button. [The same instance of the CWB from above is put back into the same Border on the right]

I'm not entirely sure why but the issue doesn't happen if there is only 1 CWB. e.g. if you comment out the code in the ctor to create the left hand instance.

As a side note I'm wondering if there is another (unrelated) bug here in that the offscreen browser is not recreated if the HwndSource is different. It seems like when it's initially created its Handle is provided to the CreateBrowser call. This isn't an issue for me because I'm not reparenting the control (into a different window) in my app. I just mention it since it was something I observed while debugging this.

amaitland commented 2 years ago

Need a window which will host 2 ChromiumWebBrowsers:

Please fork https://github.com/cefsharp/CefSharp.MinimalExample and push your changes to GitHub. Thanks

igandrews commented 2 years ago

Sure. I thought it might be easier to just add the window in the Wpf.Example project with the contents I provided since that's how I was debugging it with your source but if this is easier that's fine. Please see https://github.com/igandrews/CefSharp.MinimalExample/commit/f2897cbd2e28b7a29fcedbe5c905f7b50762d855

amaitland commented 2 years ago

Please see igandrews/CefSharp.MinimalExample@f2897cb

Thanks. Testing with your example and everything is working as expected. Upgraded to 97.1.10-pre and also working as expected.

What graphics card are you using? Intel Iris Xe Integrated GPU(11th Gen)?

As a side note I'm wondering if there is another (unrelated) bug here in that the offscreen browser is not recreated if the HwndSource is different

Recreating the browser would lose all state, so I don't believe this would be a desirable behaviour. Reparenting to the new Hwnd is potentially possible, though not sure that would actually give any practical benefit.

amaitland commented 2 years ago

I'm not entirely sure why but the issue doesn't happen if there is only 1 CWB. e.g. if you comment out the code in the ctor to create the left hand instance.

I suspect this is similar to #3870 which is an upstream bug in CEF.

igandrews commented 2 years ago

Thanks. Testing with your example and everything is working as expected. Upgraded to 97.1.10-pre and also working as expected.

Hmm. I'm surprised. I did notice that if I didn't let the browser load fully before doing the first remove that it may work when it was readded but it definitely doesn't work for me reliably otherwise. Will ask some others to try this on their system tomorrow to ensure this happens in other scenarios. In my actual app/usage I'm using a contentEditable div and the lack of the render means I don't see the blinking caret nor does typing occur although that happens in the sample as well because in the msn page there is an edit/input. For now then I'm just going to work around this by hooking the sourcechangedhandler and calling WasResized myself which addresses the issue.

What graphics card are you using? Intel Iris Xe Integrated GPU(11th Gen)?

Am running on a mac through parallels so not sure the graphics card will help. Seems odd that it would be the graphics card but then maybe something about the OSR I don't follow.

Recreating the browser would lose all state, so I don't believe this would be a desirable behaviour. Reparenting to the new Hwnd is potentially possible, though not sure that would actually give any practical benefit.

I wasn't advocating recreating the browser when it's hwndsource goes to null but perhaps validate that when the new hwndsource comes in that it's for the same handle and if not recreate the browser. I don't know what the browser is doing with the original window handle but if the control were reparented to another hwnd then it's not going to get window messages or state for the actual window where the render is done. Also not sure how the browser behaves if the original window is closed and its hwnd destroyed. In that case it would definitely need a new browser upon being resited. It's uncommon and it doesn't affect me so I'm not requesting any changes for it but I've written dockmanager type controls and could easily see that coming up in that kind of usage of moving from one window to another.

amaitland commented 2 years ago

Seems odd that it would be the graphics card but then maybe something about the OSR I don't follow.

Not at all. GPU's are notoriously problematic. Intel are by far the worst. There is a known issue with the Intel Irix Xe 11th Gen drivers see #3300

There's still some bugs in the CEF OSR implementation like https://github.com/cefsharp/CefSharp/issues/3870 so this might actually be one of those cases.

We shouldn't have to Resize the browser to get it to redraw in this scenario. That being said I don't have any objection to adding your workaround with a comment referencing this issue.

but perhaps validate that when the new hwndsource comes in that it's for the same handle and if not recreate the browser

As I said, recreating the browser would lose any state. Best to be avoided.

reparented to another hwnd then it's not going to get window messages or state for the actual window where the render is done.

There is by default one HWND per WPF Window . From memory PresentationSourceChanged is called when the Presentation Source (Window) changes, so changing the parent to match the Window the ChromiumWebBrowser control is attached to would likely be reasonable behaviour. I've not done any testing to see if CEF actually supports this with OSR. In WinForms, the browser is specifically parked whilst it's handle is created (if enabled).

https://github.com/chromiumembedded/cef/blob/476713095443ba5add758cb37a98f12c6e4a4bc4/include/internal/cef_win.h#L135

another hwnd then it's not going to get window messages or state for the actual window where the render is done.

In OSR it's a bit different, we effectively forward all the messages to the CefBrowser instance manually.

igandrews commented 2 years ago

Not at all. GPU's are notoriously problematic. Intel are by far the worst. There is a known issue with the Intel Irix Xe 11th Gen drivers see #3300

There's still some bugs in the CEF OSR implementation like #3870 so this might actually be one of those cases.

I understand better now. Thx.

We shouldn't have to Resize the browser to get it to redraw in this scenario. That being said I don't have any objection to adding your workaround with a comment referencing this issue.

So would you like me to submit a PR for this? At this point I've just changed my code to handle the event too and do that. Reminds me that I didn't check if my handler is called first but guessing it doesn't matter.

There is by default one HWND per WPF Window . From memory PresentationSourceChanged is called when the Presentation Source (Window) changes, so changing the parent to match the Window the ChromiumWebBrowser control is attached to would likely be reasonable behaviour. I've not done any testing to see if CEF actually supports this with OSR. In WinForms, the browser is specifically parked whilst it's handle is created (if enabled).

https://github.com/chromiumembedded/cef/blob/476713095443ba5add758cb37a98f12c6e4a4bc4/include/internal/cef_win.h#L135

I see. So the HwndSource's Handle is just used as the parent hwnd for any dialogs, context menus and other popups it might create. Ok well it's not a scenario affecting me an issue so I won't worry about it. If it matters I tried something like what I describe and the browser becomes unresponsive if the original hwnd is destroyed. If you're curious you can replace the code in the BugWindow.xaml.cs with this:

    public partial class BugWindow : Window
    {
        private CefSharp.Wpf.ChromiumWebBrowser browser;
        private Window _float;

        public BugWindow()
        {
            InitializeComponent();

            this.StaticBrowser.Child = new CefSharp.Wpf.ChromiumWebBrowser("http://www.msn.net");
        }

        private void OnAddBrowser(object sender, RoutedEventArgs e)
        {
            if (browser == null)
            {
                browser = new CefSharp.Wpf.ChromiumWebBrowser("http://www.msn.net");
            }

            if (_float == null)
            {
                _float = new Window();
                _float.Owner = this;

                this.BrowserSite.Child = null;

                _float.Content = browser;
                _float.Show();
            }
        }

        private void OnRemoveBrowser(object sender, RoutedEventArgs e)
        {
            if (_float != null)
            {
                _float.Content = null;
                //_float.Close();
                _float = null;
            }

            this.BrowserSite.Child = browser;
        }
    }

I commented out the close so you can see that the browser will work while the original parent window is open/valid. So run the sample. Click the add - this creates the browser into a new top level wpf window. Click the remove - the browser is unparented from the window and parented back into the main window. It's still functional at this time but if you close the now empty top level window that used to house it (or if you uncomment the _float.Close() in the code) then you'll see it's dead. So I'm guessing they're doing more with that hwnd than just popups. I tried to look at the cef code around the hwnd provided to setwindowless but it's hard to follow who uses that and where. I presume using a WPF Popup (which will have its own hwnd) and that would be the presentationsource for the browser) would yield the same results.

amaitland commented 2 years ago

So would you like me to submit a PR for this?

A PR would be welcome, make sure to reference this issue in a comment,

amaitland commented 2 years ago

If it matters I tried something like what I describe and the browser becomes unresponsive if the original hwnd is destroyed. If you're curious you can replace the code in the BugWindow.xaml.cs with this:

The default behaviour is to Dispose of the ChromiumWebBrowser instance when the parent window is unloaded. We need to ensure each ChromiumWebBrowser is disposed early so CEF will shutdown gracefully.

When you change the window that hosts the ChromiumWebBrowser instance you also need to assign the CleanupElement property to the new window.

Changing the CleanupElement when the presentation source changes is on my TODO list, hasn't happened yet (no issue tracking this yet).

amaitland commented 2 years ago

Changing the CleanupElement when the presentation source changes is on my TODO list, hasn't happened yet (no issue tracking this yet).

This was done in commit https://github.com/cefsharp/CefSharp/commit/1f795153597ce946b909c77f9e8e0ea5794a49a8 and is available in 97.1.61.

Please see igandrews/CefSharp.MinimalExample@f2897cb

Upgrading to 97.1.61 and still working as expected.

If you'd like to submit a PR for consideration then please do so, otherwise I'll close this.

igandrews commented 2 years ago

Sorry. I got sidetracked with other work. I'll try and submit the PR in the next couple of days. Thx

amaitland commented 2 years ago

Great, thanks 👍

amaitland commented 2 years ago

Closing as this has been inactive for some time. If this still reproduces with the current support version then please feel free to submit a PR. Thanks 👍