On Fri Dec 1 19:13:35 2023 +0000, Gabriel Ivăncescu wrote:
I understand your points, but I'm unsure how that affects the last push? I got rid of the patch that changes outer windows or keeps their refs around for longer, the other patches that are still not in !4574 were definitely fine (and even what you suggested, btw). If not, which one do you think is "not reviewable"? The only one that comes to my mind is the `doc->outer_window` change but, it's currently broken because it **does** keep ref to the outer window for "longer than it should" (i.e. goes against what you just said), and even worse it does not even hold any sort of ref, so it can access invalid memory. So all my patch does is make it become NULL when the inner window is detached, isn't that what you suggested?! The storage event patch, again, just does what you suggested: it checks if we're detached and skips sending the event altogether (I did put a FIXME comment, but that's just a comment…). Can you please tell me which patch is so wrong now?
The latest push had the same problem as previous ones: it lacked proper justification based on an analyze. Now that I filled that gap for you, we can move forward. !4574 is not a complete solution, more patches are still needed, please rebase and self re-review taking above into consideration. The patch removing outer_window could be split and misses a number of NULL checks.