weltkante
commented
4 years ago Quoting from here:
For now, the guidance from Office for Add-In developers is to use their JavaScript based Add-in engine and I haven't seen much appetite to do much with a .NET based approach. That may change in the future however, and that would be one reason to keep the status quo.
I'd prefer keeing the code not active even if you keep it around, this allows keeping it up to date during refactorings without exposing end users to defects. If VS wants to keep using this infrastructure I'd prefer option (2) so the hosting application has to opt-in.
The consensus is to leave everything as is until we get a specific scenario from a customer. If we leave the interfaces in the codebase it will be much easier to work with a customer that comes to us with a specific scenario that isn't working.
I disagree, removing or disabling ComponentManager integration until you get a scenario from a customer that requires it is the sane default for a reliable framework. WPF seems to be able to work just fine without it.
Is your feature request related to a problem? Please describe.
When WinForms is used in 3rd party hosting applications it may invoke ComponentManager interop to communicate form modality and share the message loop infrastructure with the host application. This is an undocumented COM API which to my knowledge is used by Office, VS and legacy VB6 application hosts. Activation and loading of .NET Core components into such hosts can happen by loading a COM object which happens to be implemented in .NET Core and is using WinForms for some of its UI.
Currently this kind of interop is broken and should be crashing (#247) if WinForms is invoked in an external host based on ComponentManager interop. This was an intentional choice for .NET Core 3 to make sure its obvious that this scenario is not supported, but is candidate to be fixed in some way or another for .NET Core 5
The whole ComponentManager infrastructure is undocumented and to my knowledge the Office applications are the only ones which are actively using 64bit versions. Visual Studio has only a 32bit version of the API and may have deviated from the Office declarations as far as 64bit compatibility is concerned (VS maintains its own copy of the declarations and doesn't share headers with Office). As far as I know Visual Studio is currently not using the ComponentManager interop in the designer hosting (otherwise it would probably have been running into the same issue #247). Legacy VB applications have not been supported for a long time and so far nobody has complained that a hosting VB6 application cannot load .NET Core 3 code (the inverse scenario, loading VB6 components into a .NET Core application, isn't affected by ComponentManager - it wasn't in Desktop Framework either, never has been using this code path since WinForms doesn't register its implementation for others to call)
So I believe Office is the only scenario that can currently be encountered in practice, and its already a very advanced scenario, but that doesn't mean such addins won't achieve high distribution, applications tend to just install their addins if the user doesn't opt out. Since the effects of a broken ComponentManager implementation can be very subtile and the code is hidden deep inside WinForms implementation its highly unlikely that bugs surfacing in Office COM addins will be reported back to WinForms in a timely manner, it can be years breaking many Office users experience without being root caused as caused by WinForms and fixed here. I consider it highly risky to just let refactored and completely untested code "go live" this way.
Note that only WinForms seems to have this ComponentManager integration with hosting applications, as far as I can tell WPF doesn't have a copy of this infrastructure.
PS: since this has been miscommunicated several times also note that this is about COM addins for Office, not .NET addins, even though .NET is used to implement the addin its an entirely different API model. Office currently only supports .NET addins from Desktop Framework, but that doesn't matter when C# just happens to be the language of choice for implementing a COM addin, as the .NET Core runtime supports the relevant COM interop.
Describe the solution you'd like and alternatives you've considered
I'm proposing the following options for discussion:
(1) remove the ComponentManager integration from WinForms.
This is probably the cleanest solution because it removes undocumented untestested code that cannot be maintained properly. It would make WinForms behave like WPF as far as modality of forms is concerned. This also assumes VS will not need ComponentManager integration in the future either.
If you feel bad about complete removal a variant of this option is to just remove the activation and leave the code itself in the codebase in case it may need resurrection. This allows it to be included in future refactorings and reduce the work compared to resurrecting from git history. (Can add some basic unit tests to ensure its not removed by accident as unused code.)
(2) include ComponentManager integration but making it opt-in from the application host by defining a special GUID in the service container. WinForms would look for an ComponentManager host but only use it if it also finds the marker GUID in the service container.
This would allow VS (or yet unknown hosts Microsoft may have) to opt-in into using the .NET Core ComponentManager implementation but disables it for Office and VB6 because it can't be supported. If Office in the future decides it wants special ComponentManager handling from WinForms it could add the GUID to its service container to opt-in as well, until then WinForms would act like WPF as far as modality is concerned. (This approach can also be versioned by changing the GUID you are looking for in future .NET Core versions.)
(3) include ComponentManager integration but make it opt-in from the "client" side. The developer would have to call a static Application.EnableComponentManager or similar before calling any WinForms APIs, if he doesn't WinForms wouldn't use Component Manager integration and behave like WPF as far as modality is concerned.
I don't like this option much because it puts the burden on the developer to make a decision he isn't qualified to do, the dev calling WinForms knows nothing about the compatibility between WinForms implementation internals and a third party hosting application. Still its better than the next option (4) because it gives the user choice and requires opt-in to run unreliable code if necessary for compatibility purposes.
(4) include ComponentManager integration "as is" (i.e. refactored but untested) and wait for customers to complain. This is what was proposed several times before, but I absolutely do not like this option because it makes WinForms an unreliable platform, if theres any chance please avoid this option for .NET Core 5
It is possible to gain some test coverage by consulting with VS but I do not consider this enough to make the implementation reliable, as there is no reference or documentation of the 64bit interface actually used by Office and getting it wrong is still very risky. Also getting good coverage over testing the edge cases is hard and time for .NET Core 5 is limited.
(5) wait for the Office team to tell you how the 64bit COM API is declared and perhaps give guidance how they want it to be tested to make sure WinForms integrates nicely with Office. Honestly I don't have much hope for this happening in time for .NET 5, its been about a year without that kind of feedback, just including it for completeness.
Will this feature affect UI controls?
no, but it will affect behavior of modal forms and user code calling DoEvents, as well as how the message loop is shared with respect to the host application. The effect can be very subtile which is why its important to me that untested and unreliable code does not remain active by default.
Normal .NET Core applications are not affected, they never use a hosted ComponentManager integration as the dotnet runtime host doesn't register the necessary interfaces.
/cc @RussKie @JeremyKuhne @merriemcgaw