Skip to content

Improved dispose pattern in ChromiumWebBrowser #2651

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.

Sign up for GitHub

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

Merged
amaitland merged 14 commits into from
Mar 22, 2019
Merged

Improved dispose pattern in ChromiumWebBrowser #2651

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
23e7bf5
Code Cleanup - Improved ChromiumWebBrowser Dispose/Finalise
merceyz Feb 14, 2019
57c7c9e
Wpf - Cleanup and improvements to the dispose pattern
merceyz Feb 14, 2019
de3b668
WinForms - Cleanup and improvements to the dispose pattern
merceyz Feb 14, 2019
af0997d
OffScreen - Cleanup and improvements to the dispose pattern
merceyz Feb 14, 2019
2bef6e2
Added IsDisposed to IWebBrowser
merceyz Feb 15, 2019
b3fb87b
Fixed comments
merceyz Feb 15, 2019
e8edef5
Merge branch 'master' into dispose-pattern
merceyz Feb 15, 2019
33f7b63
WinForms - Removed Finalizer and Dispose()
merceyz Feb 17, 2019
de3b5f3
Merge branch 'master' into dispose-pattern
amaitland Feb 19, 2019
5171075
Merge branch 'master' into dispose-pattern
amaitland Mar 1, 2019
2fc2d55
Merge branch 'master' into dispose-pattern
amaitland Mar 1, 2019
eefaa6e
WPF - ChromiumWebBrowser.Dispose add comment for Interlocked.CompareE…
merceyz Mar 2, 2019
b2337f9
WinForms - ChromiumWebBrowser.Dispose add comment for Interlocked.Com…
merceyz Mar 2, 2019
54901ae
Merge branch 'master' into dispose-pattern
amaitland Mar 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 46 additions & 25 deletions CefSharp.OffScreen/ChromiumWebBrowser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System;
using System.Drawing;
using System.Drawing.Imaging;
using System.Threading;
using System.Threading.Tasks;
using CefSharp.Enums;
using CefSharp.Internals;
Expand Down Expand Up @@ -43,6 +44,24 @@ public class ChromiumWebBrowser : IRenderWebBrowser
/// </summary>
private bool browserCreated;

/// <summary>
/// The value for disposal, if it's 1 (one) then this instance is either disposed
/// or in the process of getting disposed
/// </summary>
private int disposeSignaled;

/// <summary>
/// Gets a value indicating whether this instance is disposed.
/// </summary>
/// <value><see langword="true" /> if this instance is disposed; otherwise, <see langword="false" />.</value>
public bool IsDisposed
{
get
{
return Interlocked.CompareExchange(ref disposeSignaled, 1, 1) == 1;
}
}

/// <summary>
/// A flag that indicates whether the WebBrowser is initialized (true) or not (false).
/// </summary>
Expand Down Expand Up @@ -311,7 +330,7 @@ public ChromiumWebBrowser(string address = "", BrowserSettings browserSettings =
}

/// <summary>
/// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
/// Releases all resources used by the <see cref="ChromiumWebBrowser"/> object
/// </summary>
public void Dispose()
{
Expand All @@ -320,24 +339,15 @@ public void Dispose()
}

/// <summary>
/// Releases unmanaged and - optionally - managed resources.
/// Releases unmanaged and - optionally - managed resources for the <see cref="ChromiumWebBrowser"/>
/// </summary>
/// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
/// <param name="disposing"><see langword="true" /> to release both managed and unmanaged resources; <see langword="false" /> to release only unmanaged resources.</param>
protected virtual void Dispose(bool disposing)
{
// Don't reference event listeners any longer:
AddressChanged = null;
BrowserInitialized = null;
ConsoleMessage = null;
FrameLoadEnd = null;
FrameLoadStart = null;
LoadError = null;
LoadingStateChanged = null;
Paint = null;
StatusMessage = null;
TitleChanged = null;

Cef.RemoveDisposable(this);
if (Interlocked.CompareExchange(ref disposeSignaled, 1, 0) != 0)
{
return;
}

if (disposing)
{
Expand All @@ -346,18 +356,29 @@ protected virtual void Dispose(bool disposing)

if (managedCefBrowserAdapter != null)
{
if (!managedCefBrowserAdapter.IsDisposed)
{
managedCefBrowserAdapter.Dispose();
}
managedCefBrowserAdapter.Dispose();
managedCefBrowserAdapter = null;
}

// Don't reference event listeners any longer:
AddressChanged = null;
BrowserInitialized = null;
ConsoleMessage = null;
FrameLoadEnd = null;
FrameLoadStart = null;
LoadError = null;
LoadingStateChanged = null;
Paint = null;
StatusMessage = null;
TitleChanged = null;

// Release reference to handlers, make sure this is done after we dispose managedCefBrowserAdapter
// otherwise the ILifeSpanHandler.DoClose will not be invoked. (More important in the WinForms version,
// we do it here for consistency)
this.SetHandlersToNull();
}

// Release reference to handlers, make sure this is done after we dispose managedCefBrowserAdapter
// otherwise the ILifeSpanHandler.DoClose will not be invoked. (More important in the WinForms version,
// we do it here for consistency)
this.SetHandlersToNull();
Cef.RemoveDisposable(this);
}

/// <summary>
Expand All @@ -379,7 +400,7 @@ public void CreateBrowser(IWindowInfo windowInfo = null, BrowserSettings browser
{
browserSettings = new BrowserSettings();
}
else if(browserSettings.IsDisposed)
else if (browserSettings.IsDisposed)
{
throw new ObjectDisposedException("browserSettings", "The BrowserSettings reference you have passed has already been disposed. You cannot reuse the BrowserSettings class");
}
Expand Down
122 changes: 69 additions & 53 deletions CefSharp.WinForms/ChromiumWebBrowser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.ComponentModel;
using System.Drawing;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Windows.Forms;
using CefSharp.Internals;
using CefSharp.WinForms.Internals;
Expand Down Expand Up @@ -59,6 +60,25 @@ public class ChromiumWebBrowser : Control, IWebBrowserInternal, IWinFormsWebBrow
/// </summary>
private IRequestContext requestContext;

/// <summary>
/// The value for disposal, if it's 1 (one) then this instance is either disposed
/// or in the process of getting disposed
/// </summary>
private int disposeSignaled;

/// <summary>
/// Gets a value indicating whether this instance is disposed.
/// </summary>
/// <value><see langword="true" /> if this instance is disposed; otherwise, <see langword="false" />.</value>
[Browsable(false), EditorBrowsable(EditorBrowsableState.Never), DefaultValue(false)]
public new bool IsDisposed
{
get
{
return Interlocked.CompareExchange(ref disposeSignaled, 1, 1) == 1;
}
}

/// <summary>
/// Set to true while handing an activating WM_ACTIVATE message.
/// MUST ONLY be cleared by DefaultFocusHandler.
Expand Down Expand Up @@ -431,79 +451,75 @@ private void InitializeFieldsAndCefIfRequired()
}

/// <summary>
/// Releases the unmanaged resources used by the <see cref="T:System.Windows.Forms.Control" /> and its child controls and optionally releases the managed resources.
/// If not in design mode; Releases unmanaged and - optionally - managed resources for the <see cref="ChromiumWebBrowser"/>
/// </summary>
/// <param name="disposing">true to release both managed and unmanaged resources; false to release only unmanaged resources.</param>
/// <param name="disposing"><see langword="true" /> to release both managed and unmanaged resources; <see langword="false" /> to release only unmanaged resources.</param>
protected override void Dispose(bool disposing)
{
IsBrowserInitialized = false;

if (!designMode)
if (Interlocked.CompareExchange(ref disposeSignaled, 1, 0) != 0)
{
RemoveFromListOfCefBrowsers();
return;
}

//The unmanaged resources should never be created in design mode, so only dispose when
//at runtime
if (disposing && !designMode)
if (!designMode)
{
FreeUnmanagedResources();
InternalDispose(disposing);
}

// Don't maintain a reference to event listeners anylonger:
AddressChanged = null;
ConsoleMessage = null;
FrameLoadEnd = null;
FrameLoadStart = null;
IsBrowserInitializedChanged = null;
LoadError = null;
LoadingStateChanged = null;
StatusMessage = null;
TitleChanged = null;

// Release reference to handlers, make sure this is done after we dispose managedCefBrowserAdapter
// otherwise the ILifeSpanHandler.DoClose will not be invoked.
this.SetHandlersToNull();

base.Dispose(disposing);
}

/// <summary>
/// Required for designer support - this method cannot be inlined as the designer
/// will attempt to load libcef.dll and will subsiquently throw an exception.
/// Releases unmanaged and - optionally - managed resources for the <see cref="ChromiumWebBrowser"/>
/// </summary>
/// <param name="disposing"><see langword="true" /> to release both managed and unmanaged resources; <see langword="false" /> to release only unmanaged resources.</param>
/// <remarks>
/// This method cannot be inlined as the designer will attempt to load libcef.dll and will subsiquently throw an exception.
/// </remarks>
[MethodImpl(MethodImplOptions.NoInlining)]
private void RemoveFromListOfCefBrowsers()
private void InternalDispose(bool disposing)
{
Cef.RemoveDisposable(this);
}
if (disposing)
{
IsBrowserInitialized = false;

/// <summary>
/// Required for designer support - this method cannot be inlined as the designer
/// will attempt to load libcef.dll and will subsiquently throw an exception.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
private void FreeUnmanagedResources()
{
browser = null;
browser = null;

if (parentFormMessageInterceptor != null)
{
parentFormMessageInterceptor.Dispose();
parentFormMessageInterceptor = null;
}
if (parentFormMessageInterceptor != null)
{
parentFormMessageInterceptor.Dispose();
parentFormMessageInterceptor = null;
}

if (browserSettings != null)
{
browserSettings.Dispose();
browserSettings = null;
}
if (browserSettings != null)
{
browserSettings.Dispose();
browserSettings = null;
}

if (managedCefBrowserAdapter != null)
{
managedCefBrowserAdapter.Dispose();
managedCefBrowserAdapter = null;
if (managedCefBrowserAdapter != null)
{
managedCefBrowserAdapter.Dispose();
managedCefBrowserAdapter = null;
}

// Don't maintain a reference to event listeners anylonger:
AddressChanged = null;
ConsoleMessage = null;
FrameLoadEnd = null;
FrameLoadStart = null;
IsBrowserInitializedChanged = null;
LoadError = null;
LoadingStateChanged = null;
StatusMessage = null;
TitleChanged = null;

// Release reference to handlers, make sure this is done after we dispose managedCefBrowserAdapter
// otherwise the ILifeSpanHandler.DoClose will not be invoked.
this.SetHandlersToNull();
}

Cef.RemoveDisposable(this);
}

/// <summary>
Expand Down Expand Up @@ -643,7 +659,7 @@ private void CreateBrowser()
{
var windowInfo = new WindowInfo();
windowInfo.SetAsChild(Handle);

managedCefBrowserAdapter.CreateBrowser(windowInfo, browserSettings as BrowserSettings, requestContext as RequestContext, Address);

if (browserSettings != null)
Expand Down
Loading