dotnet / winforms

Windows Forms is a .NET UI framework for building Windows desktop applications.
MIT License
4.36k stars 966 forks source link

ComWrappers support #5163

Closed kant2002 closed 4 weeks ago

kant2002 commented 3 years ago

I want to raise questions about plans for adding ComWrappers support in WinForms. This make WinForms application IL trimming friendly. For me that would be big win for NativeAOT, but I understand that this is not priority. Work in .NET repository started https://github.com/dotnet/runtime/pull/54636 and I think similar approach to some degree can happens here.

I initially try to find simplest interfaces where this can be tested, but everything so intermixed with ActiveX support, so I do not know where to start. So far easiest targets to get started is following

Each these interfaces has their own challenges

IPicture, IStream, IPersistStream

Initially can be implemented for Cursor class only. IPicture used a lot in ActiveX code and that part is extremely problematic and maybe require usage of IDynamicCastable or similar complicated approach. Also probably it will require OleCreatePictureIndirect split of implementation on two parts, one for ComWrappers, second is for ComWrappers, and that would be for some time, since ActiveX seems impenetrable target for now.

IClassFactory2

This is very limited in usage and can be used for probing ground how to start make ActiveX support friendly for ComWrappers. Interface in itself not a problem, but CreateInstanceLic return object instance which as far as I understand, should be universal RCW. I personally may go with temporary using existing COM infrastructure, using Marshal.GetObjectForIUnknown until better solution arise.

IMallocSpy

This interface used only for tests, so not a big win in terms of implementation. Maybe good candidate to prepare tests for world without no built-in COM.

Testing

As mentioned in PR for System.Drawing.Common, they have to support .NET Core 3.1 and this requires a bit of changes in testing. I'm not sure how applicable this concerns for this repo, and how do you manage older .NET Core. Require discussion too.

Also a lot of tests rely on the built-in COM infrastructure so this is also should be factored somehow.

dreddy-work commented 3 years ago

@JeremyKuhne would be right person on winforms team to take this discussion further.

JeremyKuhne commented 3 years ago

We do want/intend to move to function pointers for COM, it is just a matter of resource allocation. If folks are interested in contributing to make this happen we'd welcome it. Tests will be important and depending on risk and timing some changes might have to go into .NET 7.

kant2002 commented 3 years ago

@JeremyKuhne Yes. I potentially willing to contribute in that area. I have further questions which will affect my contributions. Does this repo should take into account .NET 3.1? If yes, then since ComWrappers do not support .NET 3.1 tests should run again .NET 3.1 and NetCoreAppCurrent (?). I may need help in that area. If you have some other specifics which I may not know, would be good to have them listed.

What's better way to discuss path how to implement ComWrappers, just send PR or better start with issues?

RussKie commented 3 years ago

Does this repo should take into account .NET 3.1?

To service .NET Core 3.1 we need to meet the servicing bar, and at this stage I can't see we will. So changes will go into .NET 6.0 onwards.

What's better way to discuss path how to implement ComWrappers, just send PR or better start with issues?

Yes, a draft PR would be a good start.

Thank you

kant2002 commented 3 years ago

So changes will go into .NET 6.0 onwards.

Quick question to double-check that I understand correctly. For me this is means that I should not worry about .NET 3.1 and can just change code. No split in project files like it was in dotnet/runtime#54636. Correct? I have feeling that you answer significantly simplify my life, but cannot believe that.

JeremyKuhne commented 3 years ago

For me this is means that I should not worry about .NET 3.1 and can just change code.

Yes, we only build the next version from our main branch. @RussKie was saying that if you wanted to make the same changes to 3.1 you would have to do so in the release branch for that version and that it wouldn't be something that would meet the servicing bar.

So you do not have to worry about any other .NET versions in main other than .NET 6.

kant2002 commented 3 years ago

Most work in ComWrappers would be like in #5174 where private instances would be used to provide access to COM instances. This would work for handful of places, but not in WebBrowser. Reason for that, is that .NET allow cast __ComObject to different interfaces. In case of WebBrowser, this is utilized to have access to additional services out of ActiveX instance of WebBrowser. Example https://github.com/opentween/OpenTween/blob/55081d1f83daea1799f9e1aacecae3c1cb4578bf/OpenTween/WebBrowserController.cs#L303-L306

All options which I see is not good one.

Option 1

Claim that WebBrowser would not work in AOT scenarios. With all love which Web has it is not ideal scenario.

Option 2

Support only COM interfaces what's in WinForms on internal wrapper and turn blind eye on an issues. Not an option likely, but let's pretend that it is.

Option 3

Add additional API which will provide support for these cases. Maybe expensive.

Option 4

Get rid internally on reliance on ActiveXInstance and return __ComImport on request, essentially saying that this property is not AOT-friendly. Still unclear what to do with cases similar to OpenTween.

So I would like to at least start discussion early, so when time comes, I would be better prepared to implement that. I probably has to ask @AaronRobinsonMSFT to join discussion, because maybe I miss something which was already discussed.

Also I suppose this kind of issues may affects WebView2, but maybe it's too pessimistic to think like that.

RussKie commented 3 years ago

Claim that WebBrowser would not work in AOT scenarios. With all love which Web has it is not ideal scenario.

The WebBrowser control is all but officially deprecated, since it is based on the old IE engine. The current official guidelines are to migrate to the Edgium-based WebView2 control.

kant2002 commented 2 years ago

Problematic API which rely on the built-in COM

During my ComWrappers work, I start noticing API which make my life miserable. I would like to document them, so some solution eventually would be proposed for each of them, on case-by case basis.

public class RichTextBox
{
    // For this object would be created CCW and it should implement IRichEditOleCallback
    // Only usage on GitHub seems to be from winforms itself. I check all search results.
    protected virtual object CreateRichEditOleCallback();
}

public class HtmlHistory
{
    // IOmHistory 
    public object DomHistory { get; }
}

public class WebBrowser
{
     // WebBrowserSiteBase implements internal IDocHostUIHandler and maybe some issues arise from that
     // Have to check.
    protected override WebBrowserSiteBase CreateWebBrowserSiteBase();
}

public AxHost
{
    // IPicture returned
    protected static object GetIPictureFromPicture(Image image)

    // IPictureDisp returned
    protected static object GetIPictureDispFromPicture(Image image);

    // IPicture returned
    protected static object GetIPictureFromCursor(Cursor cursor);

    // IFont
    protected static object GetIFontFromFont(Font font);
    protected static Font GetFontFromIFont(object font);

    // IFontDisp
    protected static object GetIFontDispFromFont(Font font);
    protected static Font GetFontFromIFontDisp(object font);
}

Will update this message when I found more of these classes.

kant2002 commented 2 years ago

Another big area for ComWrappers - hosting of ActiveX controls The ActiveX host know about following OLE interfaces

Ole32.IPersistStreamInit
Ole32.IPersistStream
Ole32.IPersistStorage
Ole32.IPersistStreamInit
Ole32.IQuickActivate
Ole32.IOleControl
Ole32.IOleInPlaceActiveObject
Ole32.IOleInPlaceObject
Ole32.IOleInPlaceObjectWindowless
Oleaut32.IPersistPropertyBag
VSSDK.ICategorizeProperties

I think for handling this case should be created separate ActiveXComWrappers class which will creates object implementing IDynamicInterfaceCastable which mimic very closely what's happening here. Sample which show how it can work - https://github.com/dotnet/samples/tree/main/core/interop/IDynamicInterfaceCastable

weltkante commented 8 months ago

10583 will probably contribute some work here, I'm prototyping ActiveX support where the client is implemented using COM source generators (i.e. based on ComWrappers) and this is currently broken

JeremyKuhne commented 1 month ago

@kant2002 can you clarify what exactly remains here? We're not using built-in COM at all now afaik.

dotnet-policy-service[bot] commented 1 month ago

This submission has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 14 days.

It will be closed if no further activity occurs within 7 days of this comment.