Drop application handler on run loop exit

[madsmtm]

Part of #2903, split out from #3895. This is the motivation for doing #3721.

Calling the Drop impl of the user's ApplicationHandler is important on macOS, iOS and Web, since they don't necessarily return from EventLoop::run_app. This PR fixes that.

And now that we reliably call Drop, the ApplicationHandler::exited event/callback is unnecessary; using Drop composes much better (open files etc. stored in the app state will be automatically flushed), and prevents weirdness like attempting to create a new window while exiting. So this PR also removes that method.

• Tested on all platforms changed
  • macOS
  • iOS
  • Web
  • Wayland/X11/Windows/Android are trivial
• Added an entry to the changelog module if knowledge of this change could be valuable to users
• Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
• Created or updated an example program if it would help users understand this functionality

[kchibisov]

I'm in favor of doing so, however there's a case for rnn_app_on_demand and pump_app_events API.

What do you think about keeping the exiting callback, so users have a special place to run maybe some fail-able logic and e.g. clean-up things in case of on_demand/pump_event, but also guarantee to call drop on the State when we consume user state?

Also, there're cases where the user doesn't really want to drop their entire state in case of e.g. run_on_demand, but they may want to drop only part of it.

To sum up, for run_app the drop is fine, probably. For run_on_demand and pump_app_events they might need an exiting callback, which I think, is fine to keep anyway, just for compat sake.

[madsmtm]

For run_app_on_demand and pump_app_events, the user is free to pass &mut app; that way, their state won't get dropped. For both of these, they'll be able to "detect" the exiting of the event loop by simply running code after:

let mut app = App::default();
event_loop.run_app_on_demand(&mut app)?;
// exited()
event_loop.run_app_on_demand(&mut app)?;
// exited()

loop {
    let status = event_loop.pump_app_events(Some(Duration::ZERO), &mut app);

    if let PumpStatus::Exit(exit_code) = status {
        // exited()
        break ExitCode::from(exit_code as u8);
    }

    sleep(Duration::from_millis(16));
}

[kchibisov]

I've also thought about this, however run_app_on_demand implies that user should clear the state related to windows before the loop returns back to the user. It could be done with other events though, but I generally don't see an issue with keeping the event. Though, it could be a general run_app_on_demand issue that it doesn't ensure that your window will be closed, or that it closes all windows, etc, etc.

For pump_events it'll likely fine though, the api a bit sketchy.

[madsmtm]

I've also thought about this, however run_app_on_demand implies that user should clear the state related to windows before the loop returns back to the user.

Hmm, is that actually required by any backends? We don't drop the window in the run_on_demand.rs example.

If so, this could still be handled by the user with:

struct PartialApp<'app> {
    app: &'app App,
    windows: Vec<Box<dyn Window>>,
}

let mut app = App::default();
let partial_app = PartialApp { persistent: &mut app, windows: vec![] };
event_loop.run_app_on_demand(partial_app)?;

let partial_app = PartialApp { persistent: &mut app, windows: vec![] };
event_loop.run_app_on_demand(partial_app)?;

Of course, that's more verbose, but IMO also clearer what happens state-wise.

It could be done with other events though

Indeed, window cleanup should generally be done in WindowEvent::CloseRequested/WindowEvent::Destroyed. I have a larger issue about this in draft (we need WindowEvent::Created too), but of course that's all smokes and mirrors until I get around to posting it ;).

[madsmtm]

Responding to #4146 (comment) (posted in the wrong place).

Hmm, is that actually required by any backends? We don't drop the window in the run_on_demand.rs example.

On macOS it simply doesn't really work if you use run_app_on_demand and don't wait for all windows to close(macOS tells you that your app is stuck). In general, we kind of say that everything should be dropped in the docs.

See #4135

I suspect that's just a bug of the current implementation (run_app_on_demand is kinda hackily written NVMD, I think it's fine, it's the pump_app_events impl that I have a problem with), but I guess we can wait until that's resolved.

[madsmtm]

Looked in to #4135 now, and we can indeed do the cleanup correctly there, by force-closing windows before exiting (which will trigger WindowEvent::Destroyed and give the user extra time to respond if they need it).

[madsmtm]

Notes from meeting: It may have issues with the display needing to be shut down between run_app_on_demand, but it should be fine with well-behaved display servers. So probably fine to move forwards with this.