Ensure uniqueness of window state keys

[savetheclocktower]

The above issue has been open for a while. Investigation revealed a strange set of assumptions in how project windows are serialized and deserialized.

Pulsar keeps track of window states in the aptly named StateStore — an IndexedDB or SQLite database (depending on your configuration) that is shared across the entire application. Changes to the state of the window are serialized and written to the database so that the state can be restored on your next launch. This means, among other things, that each window must have its own unique identifier.

If we were doing this from scratch, we’d probably just have that identifier be randomly created. But instead, the Atom developers made it so that the “state key” (as I’ll call it henceforth) was deterministically generated based on the project root(s).

The upside of this is that one window can “adopt” another window’s state. Suppose a user created a new empty window, then added path Y as a project root. If a state already existed for a previous window that had only Y as a project root, we could grab that state and offer to bring it into this existing stateless window. (We prompt the user about this and let them decide.)

This is not my favorite feature — but then not all features need to please me personally. Still, it means that two different windows will actually share the same state key if their project roots are the same!

This is not a crazy edge case. The tab view has a context menu option named “Open in New Window” that will open a file in a new project window rather than a tab in the existing window. Selecting that option results in a cloning of the existing window in nearly all respects, including project roots.

This is the root cause of the symptoms described in #1204 — three separate windows trying to share the same state key. Once we guarantee that each will have its own, they’ll be able to serialize and deserialize without stepping on one another.

In order to accomplish this with as little disruption as possible, we'll try to deliver the best of both worlds here:

1. When a new window first determines its state key, it should be deterministically computed based on the project root(s).
2. If another window is opened that has the same project roots as the first, it should instead get a randomly generated state key. This means it can’t be used in the “adoption” scenario above, since its key isn't guessable — but the first window can, so that’s not a big deal.
3. If the first window were to close before the second window opened, then there wouldn’t be a conflict; the second window could reuse the deterministic state key of the first.

---

This is new territory because it required new tests in main-process. Amazingly, I discovered a pre-existing failing test in that code — which implies that we aren’t actually running the main process tests in CI! This would’ve been an easy oversight when writing our GHA-based CI back in the early Pulsar days, since main process specs have a different test runner and are spawned with a different command.

I’m opening this PR as a draft for now so that I can get a field report from @ConnorS-P. But if this proposed fix seems to work, then I’ll see if I can update this PR so that it also alters the CI jobs to run the main process tests. (Compared to the renderer tests, main process tests are quick and rather simple.)

[savetheclocktower]

I'm giving this the 1.132.0 milestone, but I could see it slipping to 1.133.0 if we run into complications.

[savetheclocktower]

Actually, this isn't quite working right for me locally, despite all the tests passing. Bear with me!

[savetheclocktower]

This isn't exactly a dud of a PR, but it does need some work:

• There's a spec in atom-environment.spec.js that will almost certainly have to be changed. Currently it reaches deep into the implementation details to assert that the state key is generated entirely based on project paths. (EDIT: Fixed.)
• If you've set core.restorePreviousWindowsOnStart to always, then when Pulsar isn't running and you typepulsar ., you'll open the current folder in a new window alongside whatever was already present from your previous session. If your previous session happened to include a project with the current directory as its only root, then you've now got two copies of the same project; somehow it does the right thing and merges them into one window, but you get the awkward “add to this window and discard state”/“open in new window and recover state” dialog when you didn't before. I'll look at this more deeply later. (EDIT: this seems to be fixed after a couple of changes.)
• Even if you're just restoring the previous state without opening a new folder (no path argument when launching via CLI), the first window you see is still using a randomly generated state key, even though it should not. This is certainly down to main process window management code that I do not yet understand. (EDIT: Fixed.)

[savetheclocktower]

Also, I found another fun thing:

• There's an “Open in New Window” context menu option in the tree view that, when used, will open a new window with no project roots and open your file for editing within that window.
• There's a different “Open in New Window” context menu option on a tab that, when used, will open a new window with the same project roots as the current window and open your file for editing within that window.

These should probably be harmonized so that they do the same thing.

[ConnorS-P]

lmk if you want me to test this.

[savetheclocktower]

lmk if you want me to test this.

Yeah, if you're willing! Since you're on Linux x86, this job should have artifacts you can use to verify this. Thanks!

[ConnorS-P]

Ok, well I already hit one issue. I opened up my project, right clicked a tab and hit open in new window, got hit with this:

If I hit open in new window and recover state, I get stuck in a loop of it popping up with this same thing again. If I hit the other option it works almost as intended. It opens the file I selected, but also all of my other open files, which I don't think is intended.

BUT AS FOR THE IMPORTANT PART

I can close pulsar and it DOES remember my windows state correctly across multiple windows!

[savetheclocktower]

Progress!

I don't know when I'll have time to tweak it further, but I have assigned this to a milestone — which means that it won't fall through the cracks. If It fails to make it into 1.132, I'll assign it a later milestone rather than remove the milestone altogether.

[ConnorS-P]

Cool. Thank you!

[savetheclocktower]

Sounds like we're close to cracking this, but that dumb add-previous-state-to-this-window dialog (which I've never liked!) is getting flummoxed.

We're shipping 1.132.0 soon, so I've kicked this back to the 1.133.0 milestone.