Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash in OnLostKeyboardFocus #460

Closed
sycsoft opened this issue Aug 25, 2014 · 7 comments
Closed

Crash in OnLostKeyboardFocus #460

sycsoft opened this issue Aug 25, 2014 · 7 comments

Comments

@sycsoft
Copy link

sycsoft commented Aug 25, 2014

In all place where you used "managedCefBrowserAdapter", you need do check like below:
if (!(managedCefBrowserAdapter != null && !managedCefBrowserAdapter.IsDisposed))
return;
This is because, in WPF, if I put the UI in a another thread, the event will still happen after disposed, so "managedCefBrowserAdapter" will become null or disposed.
private void OnLostKeyboardFocus(object sender, KeyboardFocusChangedEventArgs e)
{
managedCefBrowserAdapter.SendFocusEvent(false);
}

@sycsoft
Copy link
Author

sycsoft commented Aug 25, 2014

Sorry to add, it is in CefSharp.Wpf project.

@amaitland
Copy link
Member

@sycsoft Can you clarify what you mean by the following?

This is because, in WPF, if I put the UI in a another thread, the event will still happen after disposed, so "managedCefBrowserAdapter" will become null or disposed.

Just a little nitpick, the code you provided as an example can be simplified as

if (managedCefBrowserAdapter == null || managedCefBrowserAdapter.IsDisposed))
    return;

@amaitland
Copy link
Member

Possibly even provide a sample demonstrating the problem your having? Under a typical WPF usage scenario e.g. MVVM based UI the current implementation works as expected.

@sycsoft
Copy link
Author

sycsoft commented Aug 26, 2014

I tried, but unfortunately, I put this control into a complex WPF UI framework, I cannot reproduce it in simple demo, maybe I did not catch the point where it happens, but anyway I fix it by invoke Keyboard.ClearFocus() before dispose the browser, thanks.

@amaitland
Copy link
Member

@sycsoft Can you apply this change locally and see if that resolves your problem in a generic sense?

amaitland@edf100c

I'd happily see the events unhooked when we start disposing the control.

@sycsoft
Copy link
Author

sycsoft commented Aug 27, 2014

I think I got the reason, seems when disposing, we dispose disposable objects then remove hook, but I checked "managedCefBrowserAdapter" will be disposed before remove hook, so this make it still possible to still send event before remove hook, I put the remove hook prior of clear disposable objects, it fixed my issue, but I am not sure if it will cause other issue, I think you will take care of this:).
protected virtual void Dispose(bool isdisposing)
{
Cef.RemoveDisposable(this);

        RemoveSourceHook(); //New place

        foreach (var disposable in disposables)
        {
            disposable.Dispose();
        }
        disposables.Clear();

        //RemoveSourceHook(); //Old place

        DoInUi(() => WebBrowser = null);
        managedCefBrowserAdapter = null;
        ConsoleMessage = null;
        FrameLoadStart = null;
        FrameLoadEnd = null;
        LoadError = null;
    }

@amaitland
Copy link
Member

Fix for this has been merged into master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants