-
Notifications
You must be signed in to change notification settings - Fork 199
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
Implement ComWrappers.RegisterForTrackerSupport to be able create CCW #1544
Conversation
This moves a bit towards dotnet#306 and dotnet#1453
...lr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.CoreRT.cs
Outdated
Show resolved
Hide resolved
...lr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.CoreRT.cs
Outdated
Show resolved
Hide resolved
…e/InteropServices/ComWrappers.CoreRT.cs Co-authored-by: Jan Kotas <[email protected]>
...lr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.CoreRT.cs
Show resolved
Hide resolved
…e/InteropServices/ComWrappers.CoreRT.cs
@@ -364,6 +489,9 @@ public object GetOrRegisterObjectForComInstance(IntPtr externalComObject, Create | |||
if (flags.HasFlag(CreateObjectFlags.Aggregation)) | |||
throw new NotImplementedException(); | |||
|
|||
if (flags.HasFlag(CreateObjectFlags.TrackerObject)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to make sure that I understand this right: This blocks the newly added code from getting executed for now. Is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I implement only CCW. I thought that throw that for RCW was a right way, but if I remove that lines, then app from #1453 is basically working (except it does not produce any Toast for me). But exit code == 0. So I would remove these lines, and let it leak. At least you wasn't agains that previously.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This moves a bit towards #306 and #1453