search

VsixCommunity / Community.VisualStudio.Toolkit

Making it easier to write Visual Studio extensions
Other
256 stars 44 forks source link

Some tool windows are broken if using VS.Events.WindowEvents.ActiveFrameChanged #455

Open lordfanger opened 1 year ago

lordfanger commented 1 year ago

During investigating the issue in another repository, I've identified the problem there. (or can be somewhere deeper inside VS)

The constructor ActiveFrameChangeEventArgs(IVsWindowFrame oldFrame, IVsWindowFrame newFrame) calls constructors for WindowFrame(IVsWindowFrame frame) and these in turn calls SetProperty(-3011, this) later invoking NotifyPropertyChanged("ViewHelper") in original WindowFrame possibly messing other things during Window initialization. My investigation ended here, I didn't went further.

Anyway, I was able to reproduce the bug with calling WindowFrame(IVsWindowFrame frame) myself.

VS.Events.WindowEvents.FrameIsVisibleChanged += args =>
{
    _ = new WindowFrame(args.Frame);
}

Is it possible to not create new instances and reuse the old ones when they are already WindowFrame?

madskristensen commented 1 year ago

Thanks for looking into this.

What if it didn't new up a WindowFrame at all, but just returned the IVsWindowFrame. Then we can add a extension method to IVsWindowFrame that returns a WindowFrame for people to use when needed. It can return null if the WindowFrame constructor fails.

lordfanger commented 1 year ago

Wouldn't changing signature break all current usages?

In current WindowManagerService implementation they are guaranteed to be WindowFrame or null.

WindowFrame oldFrame = varValueOld as WindowFrame;
WindowFrame newFrame = varValueNew as WindowFrame;
NotifyWindowFrameEvent(delegate(uint cookie, IVsWindowFrameEvents events)
{
    events.OnActiveFrameChanged(oldFrame, newFrame);
});

But with extension way. WindowFrame constructor should be called on main thread, maybe the async method should be created as well.