dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
7.08k stars 1.17k forks source link

[API Proposal]: Expose `HwndWrapper.AddHookLast` to external callers in `HwndSource` #8956

Open mgaffigan opened 7 months ago

mgaffigan commented 7 months ago

Background and motivation

WPF permits window hooks which run before HwndTarget handles messages, but does not provide any way to handle messages after HwndTarget has processed the normal messages. This prevents replacing DefWindowProc with DefMDIChildProc or editing the result from DefWindowProc (e.g.: WM_NCHITTEST).

Calling AddHookLast by reflection verifies this new API is apparently sufficient to permit an HwndSource to act as an MDI Child.

API Proposal

public class HwndSource
{
    public void AddHookLast(HwndSourceHook hook);
}

Sample implementation which neglects removal: https://github.com/mgaffigan/WpfMdiChildSample/blob/master/WpfMdiClientWindow/HwndSourceHacks.cs#L18

API Usage

A full example is listed at https://github.com/mgaffigan/WpfMdiChildSample

var winWPFContent = new HwndSource(/* snip setting up window with WS_EX_MDICHILD */);
winWPFContent.AddHookLast(MdiChildHook);
winWPFContent.RootVisual = new UserControl1();

private static nint MdiChildHook(nint hwnd, int msg, nint wParam, nint lParam, ref bool handled)
{
    handled = true;
    return DefMDIChildProcW(hwnd, msg, wParam, lParam);
}

Alternative Designs

For the specific use case of enabling MDI Child Windows implemented in WPF, WinForms with ElementHost may be used with additional considerations.

Reimplementation of HwndTarget message handling in user code is likely to result in hard-to-diagnose subtle bugs.

Risks

None identified due to opt-in and obscure use of the API.

The reference implementation linked above does not permit removal of a hook, which may violate expectations given the presence of HwndSource.RemoveHook.

lindexi commented 7 months ago

@mgaffigan Thank you for your suggestion. However, I believe that having a layered structure for APIs is a good design approach. For some lower-level APIs that could potentially be hazardous, it might be appropriate to make them slightly more difficult to use, or require reading the documentation to understand their usage. This could encourage users to read the documentation or gain more knowledge. Of course, I totally agree that commonly used APIs or those closer to the business layer should be made more user-friendly.

mgaffigan commented 7 months ago

@lindexi, I agree these lower-level APIs are reliant upon documentation. Indeed, the style and extended style parameters remain as int rather than a flags enum, and the hook exposes unwrapped window message and pointers. Callers of HwndSource, AddHook, and the proposed AddHookLast are in P/Invoke territory.

The ask is that there be a way to hook before DefWondowProc. In WPF it is impossible for a component author to fix since the required APIs are inaccessible. Is there a supported way that I am missing?

I'm not opinionated on the shape of the API, though AddHookLast does seem congruent to the existing AddHook.

miloush commented 7 months ago

Why would the hook not be removed by RemoveHook? It doesn't matter which order the hooks are.

Also note that the proposal is to change an existing internal method into a public one. There already is an existing method with the same signature that adds the hook first, so there isn't really any API structure to change, it should be consistent.

There is some notion of a use case, as the method is/was needed internally.

This prevents replacing DefWindowProc with DefMDIChildProc or editing the result from DefWindowProc (e.g.: WM_NCHITTEST).

Is that correct? Can you not just set it as handled to replace it, or call it yourself to edit the result? I am also a bit confused about how putting it first or last in this list makes it go before or after the default one. Maybe I am looking in the wrong place, but the added hooks go on the null place here:

https://github.com/dotnet/wpf/blob/3c8f486a72db65d2574fedbc2617c9d9d21abefc/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/InterOp/HwndSource.cs#L238

That would only make difference if the default one was also added using AddHook, which i cannot quickly see where it happens.

mgaffigan commented 7 months ago

@miloush, I agree that the hook should be removed by RemoveHook. My comment about removal was that the code I provided as an example implementation does not permit removal of the hook. That is a defect of the example implementation, not of the API shape. It is trivially resolvable.

AddHookLast is needed separate from AddHook since they are on either side of the default implementation of HwndSource. The code you quoted (HwndSource.cs:238) adds the hooks in reverse order, so the resultant "stack" is:

  1. _publicHook / HwndSource.PublicHooksFilterMessage which calls the hooks added by HwndSource.AddHook (lazily created only if AddHook is called or passed via ctor)
  2. _inputHook / HwndSource.InputFilterMessage
  3. _layoutHook / HwndSource.LayoutFilterMessage
  4. _hwndTargetHook / HwndSource.HwndTargetFilterMessage
  5. Implicit DefWindowProc via HwndSubclass.DefWndProcWrapper (via HwndWrapper)

The goal is to keep the default input, layout, and HwndTarget handling, but replace other unhandled messages. This is permitted by the HwndTarget.AddHookLast internal API to add a hook after 4 but before 5 in the above list.

  1. _publicHook / HwndSource.PublicHooksFilterMessage / AddHook
  2. _inputHook / HwndSource.InputFilterMessage
  3. _layoutHook / HwndSource.LayoutFilterMessage
  4. _hwndTargetHook / HwndSource.HwndTargetFilterMessage
  5. AddHookLast
  6. Implicit DefWindowProc via HwndSubclass.DefWndProcWrapper

It is not possible to use a hook at the top of the stack (AddHook) since

Only AddHookLast permits replacement of DefWindowProc by returning handled == true on every message, since the other hooks have already filtered those messages that they respond to.