Fix the application not disposing by breaking circular references on quit

[robert-ancell]

GtkApplication windows contain a reference back to the application. This means the MyApplication object in the Flutter application never disposes. We workaround this by removing these references before we quit. This occurs for all GTK applications that quit using this method, see https://gitlab.gnome.org/GNOME/gtk/-/issues/6190. This may be fixed upstream at some point but the proposed workaround should solve the problem for all versions and not conflict with any upstream solution.

Fixes flutter/flutter#136582

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie or stuartmorgan on the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[gspencergoog]

Is there any way to test this? Seems like it would be a good thing to prevent a regression here.

[cbracken]

lgtm! As @gspencergoog notes, if there's a reasonable test that could be written, please add it.

Based on the issue filed, it sounds like perhaps a solution can eventually be upstreamed to GTK?

[robert-ancell]

To test we'd have to launch and close a full Flutter application - it wasn't clear to me how we'd do that.

I hope an upstream fix does end up in GTK, but we wouldn't be able to rely on it being there for quite some time. I talked to a GTK maintainer and they didn't see it as a major issue and they expected apps to use the shutdown signal.

[cbracken]

To test we'd have to launch and close a full Flutter application - it wasn't clear to me how we'd do that.

At some point I should get back to adding support for fixture tests for the Linux embedder. Until then, I don't think we have support for testing this.

[chinmaygarde]

adding support for fixture tests for the Linux embedder

Can we file a bug for this and a test exemption request on Discord to continue?

[cbracken]

We're waiting on a test to be written; there have been side discussions on a mechanism for testing this and @gspencergoog has landed code that should enable a test for this, so in all likelihood won't need to file for a test exemption.

[chinmaygarde]

Any updates on a test so we can move this along?

[robert-ancell]

Has a test now. @gspencergoog could you have a check if it works with the testing framework changes you made? In particular I call gtk_init() without any issues, I see you use fl_ensure_gtk_init in testing/fl_test_gtk_logs.h. I'm not sure if that should be refactored into a separate module or I should be using that.

[gspencergoog]

fl_ensure_gtk_init is mainly used to make sure that the log message handler is set. If gtk_init is OK to call multiple times, then you don't have to use fl_ensure_gtk_init, but you'll need to if you have tests that look for critical log messages. That function exists because calling g_log_set_writer_func more than once is not a good idea (it asserts). Perhaps the name could have been better (e.g. fl_ensure_log_set_writer_func). I guess I called it that because calling a function with "init" in the name more than once seems like a bad idea.