Skip to content

[release/3.1] Prevent NULL HWND's from being parented under SystemResources listener windows #2101

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
4 commits merged into from
Oct 29, 2019
Merged

[release/3.1] Prevent NULL HWND's from being parented under SystemResources listener windows #2101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
86347e9
When an `HwndHost `receives `SourceChanged `event, it goes through `B…
vatsan-madhavan Oct 23, 2019
568e4d8
Merge branch 'release/3.1' into systemresources-hwnd-fixup-rel31
vatsan-madhavan Oct 29, 2019
fc2c6a8
Merge branch 'release/3.1' into systemresources-hwnd-fixup-rel31
vatsan-madhavan Oct 29, 2019
fa5ca3d
Merge branch 'release/3.1' into systemresources-hwnd-fixup-rel31
vatsan-madhavan Oct 29, 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
29 changes: 17 additions & 12 deletions src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Interop/HwndHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -976,23 +976,28 @@ private void BuildOrReparentWindow()
UnsafeNativeMethods.SetParent(_hwnd, new HandleRef(null,hwndParent));
}
}
else
else if (_hwnd.Handle != IntPtr.Zero)
{
// Reparent the window to notification-only window provided by SystemResources
// This keeps the child window around, but it is not visible. We can reparent the
// window later when a new parent is available
var hwnd = SystemResources.GetDpiAwarenessCompatibleNotificationWindow(_hwnd);
UnsafeNativeMethods.SetParent(_hwnd, new HandleRef(null, hwnd.Handle));
// ...But we have a potential problem: If the SystemResources listener window gets
// destroyed ahead of the call to HwndHost.OnDispatcherShutdown(), the HwndHost's window
// will be destroyed too, before the "logical" Dispose has had a chance to do proper
// shutdown. This turns out to be very significant for WebBrowser/ActiveXHost, which shuts
// down the hosted control through the COM interfaces, and the control destroys its
// window internally. Evidently, the WebOC fails to do full, proper cleanup if its
// window is destroyed unexpectedly.
// To avoid this situation, we make sure SystemResources responds to the Dispatcher
// shutdown event after this HwndHost.
SystemResources.DelayHwndShutdown();
Debug.Assert(hwnd != null);
Trace.WriteLineIf(hwnd == null, $"- Warning - Notification Window is null\n{new System.Diagnostics.StackTrace(true).ToString()}");
if (hwnd != null)
{
UnsafeNativeMethods.SetParent(_hwnd, new HandleRef(null, hwnd.Handle));
// ...But we have a potential problem: If the SystemResources listener window gets
// destroyed ahead of the call to HwndHost.OnDispatcherShutdown(), the HwndHost's window
// will be destroyed too, before the "logical" Dispose has had a chance to do proper
// shutdown. This turns out to be very significant for WebBrowser/ActiveXHost, which shuts
// down the hosted control through the COM interfaces, and the control destroys its
// window internally. Evidently, the WebOC fails to do full, proper cleanup if its
// window is destroyed unexpectedly.
// To avoid this situation, we make sure SystemResources responds to the Dispatcher
// shutdown event after this HwndHost.
SystemResources.DelayHwndShutdown();
}
}
}
finally
Expand Down
48 changes: 20 additions & 28 deletions src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/SystemResources.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1023,7 +1023,7 @@ internal static void OnThemeChanged()
/// This is the default HWND used to listen for theme-change messages.
///
/// When <see cref="IsPerMonitorDpiScalingActive"/> is true, additional notification windows are created
/// on-demand by <see cref="EnsureResourceChangeListener(DpiAwarenessContextValue)"/> or <see cref="EnsureResourceChangeListener(DpiUtil.HwndDpiInfo)"/>
/// on-demand by <see cref="EnsureResourceChangeListener(DpiUtil.HwndDpiInfo)"/>
/// as the need arises. For e.g., when <see cref="System.Windows.Interop.HwndHost"/> calls into <see cref="GetDpiAwarenessCompatibleNotificationWindow(HandleRef)"/>,
/// we would look for a notify-window that matches both (a) DPI Awareness Context and (b) DPI Scale factor of the foreign window from HwndHost to return. If none is found,
/// we would create one and add it to our list in <see cref="_hwndNotify"/> and return the newly created notify-window.
Expand Down Expand Up @@ -1054,27 +1054,6 @@ private static void EnsureResourceChangeListener()
}
}

/// <summary>
/// Ensures that the notify-window corresponding to a given <paramref name="dpiContextValue"/> has been
/// created.
/// </summary>
/// <param name="dpiContextValue">DPI Awareness Context for which notify-window has to be ensured</param>
private static void EnsureResourceChangeListener(DpiAwarenessContextValue dpiContextValue)
{
EnsureResourceChangeListener();

// Test if _hwndNotify has a key that contains dpiContextValue - otherwise create and add a notify-window with
// this DPI Awareness Context
if (_hwndNotify.Keys.FirstOrDefault((hwndDpiContext) => hwndDpiContext.DpiAwarenessContextValue == dpiContextValue) == null)
{
var hwndDpiInfo = CreateResourceChangeListenerWindow(dpiContextValue);
if (!_dpiAwarenessContextAndDpis.Contains(hwndDpiInfo))
{
_dpiAwarenessContextAndDpis.Add(hwndDpiInfo);
}
}
}

/// <summary>
/// Ensures that a notify-window corresponding to a given HwndDpiInfo(=DpiAwarenessContextValue + DpiScale) has been
/// created.
Expand All @@ -1085,23 +1064,32 @@ private static void EnsureResourceChangeListener(DpiAwarenessContextValue dpiCon
/// whose characteristics we are trying to match, we are guaranteed that the DPI Scale factor of the newly created HWND
/// would be identical to that of the reference HWND.
/// </remarks>
private static void EnsureResourceChangeListener(DpiUtil.HwndDpiInfo hwndDpiInfo)
private static bool EnsureResourceChangeListener(DpiUtil.HwndDpiInfo hwndDpiInfo)
{
EnsureResourceChangeListener();

// It's meaningless to ensure RCL for Invalid DACV
if (hwndDpiInfo.DpiAwarenessContextValue == DpiAwarenessContextValue.Invalid)
{
return false;
}

if (!_hwndNotify.ContainsKey(hwndDpiInfo))
{
var hwndDpiInfoKey =
CreateResourceChangeListenerWindow(
hwndDpiInfo.DpiAwarenessContextValue,
hwndDpiInfo.ContainingMonitorScreenRect.left,
hwndDpiInfo.ContainingMonitorScreenRect.top);
Debug.Assert(hwndDpiInfo == hwndDpiInfoKey);

if (!_dpiAwarenessContextAndDpis.Contains(hwndDpiInfo))
if (hwndDpiInfoKey == hwndDpiInfo && // If hwndDpiInfoKey != hwndDpiInfo, something is wrong, abort.
!_dpiAwarenessContextAndDpis.Contains(hwndDpiInfo))
{
_dpiAwarenessContextAndDpis.Add(hwndDpiInfo);
}
}

return _hwndNotify.ContainsKey(hwndDpiInfo);
}

/// <summary>
Expand All @@ -1113,7 +1101,7 @@ private static void EnsureResourceChangeListener(DpiUtil.HwndDpiInfo hwndDpiInfo
/// <param name="y">y-coordinate position on the screen where the window is to be created</param>
/// <remarks>
/// Assumes that <see cref="_hwndNotify"/> and <see cref="_hwndNotifyHook"/> have been initialized. This method
/// must only be called by <see cref="EnsureResourceChangeListener"/> or <see cref="EnsureResourceChangeListener(DpiAwarenessContextValue)"/>
/// must only be called by <see cref="EnsureResourceChangeListener"/> or <see cref="EnsureResourceChangeListener(DpiUtil.HwndDpiInfo)"/>
/// </remarks>
/// <returns><see cref="DpiUtil.HwndDpiInfo"/> of the newly created <see cref="HwndWrapper"/>, which is also a new key added into <see cref="_hwndNotify"/></returns>
private static DpiUtil.HwndDpiInfo CreateResourceChangeListenerWindow(DpiAwarenessContextValue dpiContextValue, int x = 0, int y = 0, [System.Runtime.CompilerServices.CallerMemberName] string callerName = "")
Expand Down Expand Up @@ -1652,8 +1640,12 @@ internal static HwndWrapper GetDpiAwarenessCompatibleNotificationWindow(HandleRe
DpiUtil.GetExtendedDpiInfoForWindow(hwnd.Handle, fallbackToNearestMonitorHeuristic: true) :
new DpiUtil.HwndDpiInfo(processDpiAwarenessContextValue, GetDpiScaleForUnawareOrSystemAwareContext(processDpiAwarenessContextValue));

EnsureResourceChangeListener(hwndDpiInfo);
return _hwndNotify[hwndDpiInfo].Value;
if (EnsureResourceChangeListener(hwndDpiInfo))
{
return _hwndNotify[hwndDpiInfo].Value;
}

return null;
}

/// <summary>
Expand Down