On Sun Dec 3 10:28:27 2023 +0000, Jacek Caban wrote:
The more I look at it the more I'm convinced that the change to keep outer window reference is not worth it. Storage events seem just outright broken on native, so that's not a good reason. Looking at Gecko, I can see that it does what we currently do and resets outer window pointer from inner window and document when inner window changes. It means that whatever we do on Wine side, everything that depends on Gecko will still not "work". The change is also quite invasive. It changes a meaning of pointers that are used module-wide, so there is some risk associated with it and the motivation seems questionable to me. Why do you need that in the first place? This is not something you've actually seen mattering for real-world use case, right? Another thing about it is that it's likely to hide some problems in the future. Resetting outer pointer is nice to have a clear cut when inner window is no longer valid. It's not a strong argument, if we really have a reason to change it, we may, but I'd like to know the reason.
Well that's why I wanted it before code freeze, so any possible regressions are detected now. Isn't that the point?
For storage events, yeah I can easily get rid of the test and just keep the current "correct" behavior with a comment. I'd rather keep that than silently drop events (or even if it's logged, it's not that obvious), because that's 1) incorrect and 2) it's extremely hard to debug if something broken ever depends on it. Plus, it's not like sending the events is extra code than silently dropping them, most of the patch is in the tests.
As for the "hide problems in the future", I actually think the exact opposite. Right now, we use the document's outer_window in same way, except it's completely broken and can access invalid memory if the outer window happens to die, which would be very hard to debug (especially considering most apps using mshtml are bloated and slow with millions of frameworks).
What problem would this hide? The things that *are* different than before (e.g. parentWindow), are tested so they're fixed and correct, with the others being same as before, except not randomly crashing in some cases.
Maybe it's not by itself a strong argument, but outer_window on document is now just simply broken and I'd like to fix that before the code freeze.