Skip to content
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

Refactor PowerRename context menu registration to be reusable #1051

Closed
crutkas opened this issue Jan 7, 2020 · 9 comments
Closed

Refactor PowerRename context menu registration to be reusable #1051

crutkas opened this issue Jan 7, 2020 · 9 comments
Labels
Area-Quality Stability, Performance, Etc.

Comments

@crutkas
Copy link
Member

crutkas commented Jan 7, 2020

Other tools such as Image Resizer will need this. We should have a single base for how stuff hooks into the shell. This could be we have 2 libs for .NET and for C++ that other Toys call into but each toy shouldn't have to reimplement this work

FYI: @arjunbalgovind we'd have to migrate work toward this common base once it is online. in short term, mimic how powerrename does stuff for MSIX registration.

@crutkas crutkas added the Area-Quality Stability, Performance, Etc. label Jan 7, 2020
@crutkas crutkas added this to the 20.02 release milestone Jan 7, 2020
@arjunbalgovind
Copy link
Contributor

@crutkas Since we currently have an independently working ImageResizer, I'll first work on moving that into PowerToys and I'll take up the context menu code migration after that since the two threads of work are more or less independent.

@crutkas
Copy link
Member Author

crutkas commented Jan 7, 2020

For image Resizer, we'd migrate to this item in the future. The FYI was just to make you aware we want to have one system way to hook stuff up if at all possible.

@wjk
Copy link
Contributor

wjk commented Feb 3, 2020

Just a quick heads-up if we're going to be refactoring this: It is entirely possible to register a COM class in an MSIX package without needing to write an EXE server and calling CoRegisterClassObject(), by using the com:SurrogateServer element. I used this technique in my MSIX shell extension, and it works very well. Hope this helps!

@crutkas
Copy link
Member Author

crutkas commented Feb 3, 2020

Wonder if this will solve #1197

@enricogior
Copy link
Contributor

enricogior commented Feb 3, 2020

Hi @wjk
we didn't use that approach because it was causing the PowerRename UI to be modal (and block the Explorer UI thread). We will retry that approach, it requires a code refactoring of the PowerRename library since it was originally designed for MSI and we we use as it is we hit the bloacked UI thread issue. We will give it a try since at this stage that may solves two issues (the perf problem and the dialog not showing in the foreground).

@yuyoyuppe
Copy link
Contributor

yuyoyuppe commented Feb 4, 2020

Alright, using COM SurrogateServer instead of ExeServer didn't solve the foreground issue, since CoAllowSetForegroundWindow call from explorer.exe doesn't propagate for both of these process chains:

  • explorer => <Windows COM IPC call resolution> => dllhost.exe => PowerRenameExt.dll
  • explorer => <Windows COM IPC call resolution> => PowerRenameUWPUI.exe

@yuyoyuppe
Copy link
Contributor

@wjk we might be unaware - what are the benefits of using SurrogateServer instead of ExeServer?

@wjk
Copy link
Contributor

wjk commented Feb 4, 2020

Just the ease in writing the server code, really. In particular, .NET Core doesn't support CoRegisterClassObject(), while it can easily generate an in-proc COM loader. Other than that, I see no differences between the two approaches. Thanks!

@crutkas
Copy link
Member Author

crutkas commented Mar 17, 2020

more in terms of MSIX, closing for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Quality Stability, Performance, Etc.
Projects
None yet
Development

No branches or pull requests

5 participants