Skip to content

Conversation

@amaitland
Copy link
Member

WinForms/Wpf/OffScreen - Reorder Dispose methods

  • Set browser initialized to false
  • Release event handlers
  • Dispose managedCefBrowserAdapter
  • Set handlers to null

Follow up to #2651

- Set browser initialized to false
- Release event handlers
- Dispose managedCefBrowserAdapter
- Set handlers to null
@amaitland amaitland added this to the 73.0.0 milestone Mar 22, 2019
@amaitland amaitland requested a review from merceyz March 22, 2019 12:24
@amaitland
Copy link
Member Author

All three controls should be roughly the same in order now. Releasing the event handlers before Disposing of ManagedCefBrowserAdapter should keep the events from being fired mid Dispose.

I'm also considering freeing the handlers earlier and just setting ILifeSpanHandler = null after ManagedCefBrowserAdapter.Dispose

@AppVeyorBot
Copy link

Copy link
Member

@merceyz merceyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@merceyz
Copy link
Member

merceyz commented Mar 22, 2019

I'm also considering freeing the handlers earlier and just setting ILifeSpanHandler = null after ManagedCefBrowserAdapter.Dispose

Is it an option to set all handlers to null before that call, then have it handle the logic of closing the browser further down the stack?

@amaitland
Copy link
Member Author

Is it an option to set all handlers to null before that call

All handlers except LifeSpanHandler. It has to be freed after otherwise LifeSpanHandler.DoClose won't be called. See https://github.com/cefsharp/CefSharp/blob/cefsharp/71/CefSharp.Wpf/ChromiumWebBrowser.cs#L642

… handlers cleared

The one exception is LifeSpanHandler which is released after managedCefBrowserAdapter.Dispose
@AppVeyorBot
Copy link

@amaitland
Copy link
Member Author

LifeSpanHandler is now set to null independently of the other handlers which are set to null after the event handlers.

@amaitland amaitland merged commit c3bbd37 into cefsharp:master Mar 26, 2019
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

Successfully merging this pull request may close these issues.

3 participants