GNOME Bugzilla – Bug 764076
Add mechanism/structure to ensure all operations finish before destroying MainWindow
Last modified: 2016-06-18 08:04:58 UTC
As realized on bug #763908, we need a better mechanism/structure in place to ensure pending operations finish before destroying the MainWindow. GEdit might provide some insight on how we can accomplish this.
My current thinking is to add two methods to PhotosApplication (say photos_application_foo/unfoo - can't think of a good name right now) that look similar to gtk_application_hold/release. One of them will increase a counter and the other will decrease it. When the counter is greater than zero we should disable the app.quit GAction. The rest of the application can then wrap relevant asynchronous operations with photos_application_foo/unfoo calls. gedit is a bit more complicated because it can have multiple top-level GtkApplicationWindows, while we have only one at the moment. This can change in future when we tackle bug 758705 , but let's take one step at a time. :)
(In reply to Debarshi Ray from comment #1) > My current thinking is to add two methods to PhotosApplication (say > photos_application_foo/unfoo - can't think of a good name right now) that > look similar to gtk_application_hold/release. One of them will increase a > counter and the other will decrease it. When the counter is greater than > zero we should disable the app.quit GAction. > > The rest of the application can then wrap relevant asynchronous operations > with photos_application_foo/unfoo calls. What I don't like about this approach is that if we mess up with this "secondary" refcount, the user won't be able to close the application. Instead of disabling the app.quit action, couldn't we just hide the MainWindow, wait for the async calls to finish, and then do the quitting process as normal? That way we can even set a timeout for the async calls to finish; if they don't in that timeframe, we just ignore them and quit the app.
(In reply to Rafael Fonseca from comment #2) > (In reply to Debarshi Ray from comment #1) > > My current thinking is to add two methods to PhotosApplication (say > > photos_application_foo/unfoo - can't think of a good name right now) that > > look similar to gtk_application_hold/release. One of them will increase a > > counter and the other will decrease it. When the counter is greater than > > zero we should disable the app.quit GAction. > > > > The rest of the application can then wrap relevant asynchronous operations > > with photos_application_foo/unfoo calls. > > What I don't like about this approach is that if we mess up with this > "secondary" refcount, the user won't be able to close the application. Good point. > Instead of disabling the app.quit action, couldn't we just hide the > MainWindow, wait for the async calls to finish, and then do the quitting > process as normal? Sure, why not. In that case, the implementation of app.quit will become gtk_widget_hide followed gtk_widget_destroy if the counter is greater than zero.
Created attachment 326567 [details] [review] application: Make app.quit use the same path as the close button
Created attachment 326568 [details] [review] application: Add API to prevent the main window from getting destroyed
(In reply to Rafael Fonseca from comment #2) > (In reply to Debarshi Ray from comment #1) > > My current thinking is to add two methods to PhotosApplication (say > > photos_application_foo/unfoo - can't think of a good name right now) that > > look similar to gtk_application_hold/release. One of them will increase a > > counter and the other will decrease it. When the counter is greater than > > zero we should disable the app.quit GAction. > > > > The rest of the application can then wrap relevant asynchronous operations > > with photos_application_foo/unfoo calls. > > What I don't like about this approach is that if we mess up with this > "secondary" refcount, the user won't be able to close the application. Just for the record, the idea of disabling app.quit came from gedit's update_actions_sensitivity in gedit-window.c.
(In reply to Debarshi Ray from comment #3) > (In reply to Rafael Fonseca from comment #2) > > Instead of disabling the app.quit action, couldn't we just hide the > > MainWindow, wait for the async calls to finish, and then do the quitting > > process as normal? > > Sure, why not. In that case, the implementation of app.quit will become > gtk_widget_hide followed gtk_widget_destroy if the counter is greater than > zero. These patches are supposed to implement your idea. I have not added the part about cancelling pending operations that are taking too long to finish. At the moment we are either (a) serializing the pipeline to disk, or (b) updating the tracker database. I don't expect either of those to take too long. Things might get more interesting in the future, when we having sharing/uploads.
Review of attachment 326568 [details] [review]: ::: src/photos-application.c @@ +1784,3 @@ + + self->use_count--; + if (self->use_count == 0) We should also check self->main_window_deleted.
Created attachment 329968 [details] [review] application: Add API to prevent the main window from getting destroyed
Comment on attachment 329968 [details] [review] application: Add API to prevent the main window from getting destroyed Pushed to master. Re-open the bug or let me know otherwise if there is something wrong.