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

Add options to prevent multiple dispatcher question and setting of owner on snoop windows #73

Closed
batzen opened this issue Apr 30, 2018 · 4 comments
Assignees
Milestone

Comments

@batzen
Copy link
Collaborator

batzen commented Apr 30, 2018

Hi,

I'd like to add options to prevent the multiple dispatcher question and setting of owner on snoop windows.

Currently the question for multiple dispatchers appears every time i want to attach snoop to one of my applications and i always answer with "no" because the application only has a progress/busy indicator window running in a separate dispatcher. Getting asked every time is quite annoying. ;-)

When snoop opens it's windows it sets the owner to the first appropriate window it is able to find. If your application has multiple windows this causes snoop to be in the background when the owner is in the background and snoop being in the foreground if the owner is in the foreground. This makes interacting with multiple windows difficult. It gets even worse when you minimize the owner as snoop is also minimized then.

To give users the options to decide what should happen, I'd like to add these two options to snoops main window. That way users can decide what should happen before attaching to their application.
These options would then be saved in snoops settings file so the user doesn't have to set the options every time.

These settings would then have to be transferred from snoop to the injector launcher, from there to the injector and from there to the target process.

I already tried a few ways to transfer these options and came to the conclusion that the easiest way would be:

  • Add the options to snoops settings class (for persistence)
  • Create a new class containing the options (for transient transport)
  • Before starting the injector launcher serialize the transient options to a temporary file and pass that file to the launcher
  • Pass the file path from the launcher to the injector
  • Pass the file path from the injector to the target process by adding it to the window message being sent to the injected process
  • De-serialize the settings from the temporary file and apply the options
  • Delete the temporary file (this would be done in snoops main process after the launcher is finished)

I therefore changed the data being sent via SendMessage from a string separated by "$" to the serialized data of some new transient data class which contains the data. This will also allow us, if we ever need to, pass more data from the injector to the target process.

The SendMessage data class looks like:

public ref class InjectorData : System::Object
{
	public:
		property System::String^ AssemblyName;
		property System::String^ ClassName;
		property System::String^ MethodName;

		property System::String^ SettingsFile;
};

The data class being serialized to disk looks like this:

public sealed class TransientSettingsData
{
    private static readonly XmlSerializer serializer = new XmlSerializer(typeof(TransientSettingsData));

    public TransientSettingsData()
    {
        this.SetWindowOwner = true;
        this.UseMultipleDispatcherMode = null;
    }

    public bool SetWindowOwner { get; set; }

    public bool? UseMultipleDispatcherMode { get; set; }

    public string WriteToFile()
    {
        var settingsFile = Path.GetTempFileName();

        using (var stream = new FileStream(settingsFile, FileMode.Create))
        {
            serializer.Serialize(stream, this);	                   
        }

        return settingsFile;
    }

    public static TransientSettingsData Load(string file)
    {
        using (var stream = new FileStream(file, FileMode.Open))
        {
            return (TransientSettingsData)serializer.Deserialize(stream);
        }
    }
}

What do you think about this @cplotts @MaciekRakowski ?

@batzen
Copy link
Collaborator Author

batzen commented Jun 13, 2018

@cplotts ping

@cplotts
Copy link
Contributor

cplotts commented Jun 13, 2018

My basic reaction is that I like your proposal let's move forward with it.

What would the option be ... for the multiple dispatcher issue you are describing? I had no idea it was causing issues like that ... in the multiple dispatcher cases I have tested with, it worked pretty smoothly ... it must be due to the order the windows are created? The order that Snoop finds them in? To be honest, though, it's been some time since I have tested this scenario.

And sorry for not responding ... I'm in the middle of a big release right now and just haven't had any time to get back to Snoop after merging your initial pull request ... which is so handy, by the way. I love that you took the time to port this functionality over into Snoop.

@batzen
Copy link
Collaborator Author

batzen commented Jun 13, 2018

It's not that it doesn't work smoothly, the issue is that i don't need the multiple dispatcher mode but get asked every time i snoop the application i develop at work because it creates a persistent window on a separate dispatcher which is shown during long running tasks.

I want to address two issues with this proposal:

  1. Prevent the multiple dispatcher question
    The dispatcher question is just annoying if you always have multiple dispatchers but in 99% of all cases only want to snoop the main disptachers windows.

  2. Prevent setting the owner
    I does not matter in which order snoop tries to find the windows and sets them as it's owner. If you have two windows and none of those is modal you always run into issues.

The solution for both issues would be to add the transport for settings/options and re-work the initial snoop window. The initial snoop window would need at least the two checkboxes, maybe hidden behind an expander or something.

Would also be nice if you could have look at the other PRs i have opened. ;-)

Regarding you not responding: Absolutely no problem. I know big releases ;-) And i am very happy you like my initial PR. :-)

@batzen batzen self-assigned this Dec 15, 2018
@batzen batzen added this to the 2.11 milestone Dec 15, 2018
@batzen
Copy link
Collaborator Author

batzen commented May 27, 2019

Closed via #117

@batzen batzen closed this as completed May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants