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

improved WebView resource cleanup #291

Merged
merged 1 commit into from
Mar 26, 2014
Merged
Changes from all commits
Commits
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
72 changes: 64 additions & 8 deletions CefSharp.Wpf/WebView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class WebView : ContentControl, IRenderWebBrowser, IWpfWebBrowser
private Image image;
private Image popupImage;
private Popup popup;
private static readonly List<IDisposable> disposables = new List<IDisposable>();
private readonly List<IDisposable> disposables = new List<IDisposable>();

public BrowserSettings BrowserSettings { get; set; }
public bool IsBrowserInitialized { get; private set; }
Expand Down Expand Up @@ -83,9 +83,9 @@ public string Address

public static readonly DependencyProperty AddressProperty =
DependencyProperty.Register("Address", typeof(string), typeof(WebView),
new UIPropertyMetadata(null, OnAdressChanged));
new UIPropertyMetadata(null, OnAddressChanged));

private static void OnAdressChanged(DependencyObject sender, DependencyPropertyChangedEventArgs args)
private static void OnAddressChanged(DependencyObject sender, DependencyPropertyChangedEventArgs args)
{
WebView owner = (WebView)sender;
string oldValue = (string)args.OldValue;
Expand Down Expand Up @@ -144,6 +144,65 @@ public string Title
public static readonly DependencyProperty TitleProperty =
DependencyProperty.Register("Title", typeof(string), typeof(WebView), new PropertyMetadata(defaultValue: null));

#endregion CleanupElement dependency property

#region CleanupElement dependency property

/// <summary>
/// The CleanupElement Controls when the BrowserResources will be cleand up.
/// The WebView will register on Unloaded of the provided Element and dispose all resources when that handler is called.
/// By default the cleanup element is the Window that contains the WebView.
/// if you want cleanup to happen earlier provide another FrameworkElement.
/// Be aware that this Control is not usable anymore after cleanup is done.
/// </summary>
/// <value>
/// The cleanup element.
/// </value>
public FrameworkElement CleanupElement
{
get { return (FrameworkElement)GetValue(CleanupElementProperty); }
set { SetValue(CleanupElementProperty, value); }
}

public static readonly DependencyProperty CleanupElementProperty =
DependencyProperty.Register("CleanupElement", typeof(FrameworkElement), typeof(WebView), new PropertyMetadata(null, OnCleanupElementChanged));

private static void OnCleanupElementChanged(DependencyObject sender, DependencyPropertyChangedEventArgs args)
{
var owner = (WebView)sender;
var oldValue = (FrameworkElement)args.OldValue;
var newValue = (FrameworkElement)args.NewValue;

owner.OnCleanupElementChanged(oldValue, newValue);
}

protected virtual void OnCleanupElementChanged(FrameworkElement oldValue, FrameworkElement newValue)
{
if (oldValue != null)
{
oldValue.Unloaded -= OnCleanupElementUnloaded;
}

if (newValue != null)
{
newValue.Unloaded -= OnCleanupElementUnloaded;
newValue.Unloaded += OnCleanupElementUnloaded;
}
}

private void OnCleanupElementUnloaded(object sender, RoutedEventArgs e)
{
Cleanup();
}

protected virtual void Cleanup()
{
foreach (var disposable in disposables)
{
disposable.Dispose();
}
}

#endregion Title dependency property

#region TooltipText dependency property
Expand Down Expand Up @@ -277,10 +336,6 @@ private static void OnApplicationExit(object sender, ExitEventArgs e)
// should not explicitly have to perform this.
if (Cef.IsInitialized)
{
foreach (var disposable in disposables)
{
disposable.Dispose();
}
GC.Collect();
GC.WaitForPendingFinalizers();
Copy link
Member

Choose a reason for hiding this comment

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

Should we move these two lines also?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope the intention of this two lines is that the gc runns and cleans before CefShutdown is called. if the two lines are moved to instance cleanup then there will be a full gc cycle for each instance of webview and that just will slow things down.

Copy link
Member

Choose a reason for hiding this comment

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

OK, cool. You're right - that makes sense. Merging your fine code now - thanks!


Expand All @@ -290,10 +345,11 @@ private static void OnApplicationExit(object sender, ExitEventArgs e)

private void OnLoaded(object sender, RoutedEventArgs routedEventArgs)
{
CleanupElement = Window.GetWindow(this);
AddSourceHookIfNotAlreadyPresent();
}

public void OnUnloaded(object sender, RoutedEventArgs routedEventArgs)
private void OnUnloaded(object sender, RoutedEventArgs routedEventArgs)
{
RemoveSourceHook();
}
Expand Down